[clang][modules][deps]Remove -F option from test for clang-scan-deps #141614
Conversation
Remove the redundant -F option from test because not all platforms support -F, which is a g++ extension. The option is unnecessary because all files in the test have paths specified.
jamieschmeiser
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Jamie Schmeiser (jamieschmeiser) ChangesRemove the redundant -F option from test because not all platforms support -F, which is a g++ extension. The option is unnecessary because all files in the test have paths specified. Full diff: https://github.com/llvm/llvm-project/pull/141614.diff 1 Files Affected:
diff --git a/clang/test/ClangScanDeps/modules-pch-common-stale.c b/clang/test/ClangScanDeps/modules-pch-common-stale.c
index 100124d9ad0b0..72a4dc949b0f5 100644
--- a/clang/test/ClangScanDeps/modules-pch-common-stale.c
+++ b/clang/test/ClangScanDeps/modules-pch-common-stale.c
@@ -27,7 +27,7 @@ module mod_tu_extra { header "mod_tu_extra.h" }
// Clean: scan the PCH.
// RUN: clang-scan-deps -format experimental-full -o %t/deps_pch_clean.json -- \
-// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch -F %t \
+// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
// Clean: build the PCH.
@@ -38,7 +38,7 @@ module mod_tu_extra { header "mod_tu_extra.h" }
// Clean: scan the TU.
// RUN: clang-scan-deps -format experimental-full -o %t/deps_tu_clean.json -- \
-// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h -F %t \
+// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
// RUN: cat %t/deps_tu_clean.json | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-TU-CLEAN -DPREFIX=%/t
// CHECK-TU-CLEAN: {
@@ -94,7 +94,7 @@ module mod_tu_extra { header "mod_tu_extra.h" }
// Incremental: scan the PCH.
// RUN: clang-scan-deps -format experimental-full -o %t/deps_pch_incremental.json -- \
-// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch -F %t \
+// RUN: %clang -x c-header %t/prefix.h -o %t/prefix.h.pch \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
// Incremental: build the PCH.
@@ -107,7 +107,7 @@ module mod_tu_extra { header "mod_tu_extra.h" }
// TU that depend on modules imported from the PCH and discover the
// new dependency on 'mod_tu_extra'.
// RUN: clang-scan-deps -format experimental-full -o %t/deps_tu_incremental.json -- \
-// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h -F %t \
+// RUN: %clang -c %t/tu.c -o %t/tu.o -include %t/prefix.h \
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache
// RUN: cat %t/deps_tu_incremental.json | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-TU-INCREMENTAL -DPREFIX=%/t
// CHECK-TU-INCREMENTAL: {
|
There was a problem hiding this comment.
LGTM, thanks!
not all platforms support -F, which is a g++ extension.
It indeed looks unnecessary for this test, but what is the correct way to enable support for -F in clang tests? I would actually expect it to show up often in our tests as it's a common option for Darwin targets, but doesn't need to be strictly tied to a platform.
@jamieschmeiser can you please answer my question? |
|
I am looking into what is done elsewhere. Not relying on -F, as is now the case here, is one way (which doesn't really answer your question...) |
|
@cyndyishida The correct way to enable support for -F is to specify So, if you wish to do more than just check preprocess or check options, you should specify a darwin target. |
|
@jamieschmeiser I apologize if this is obvious, but is it a requirement that clang options follow the same conventions as gcc? If so, where is that documented? |
|
@cyndyishida From https://clang.llvm.org/compatibility.html: |
@jamieschmeiser This is where I am confused. As far as I can tell, the clang support for respecting |
|
Our legacy products had different functionality for -F than the current Clang -F functionality. See https://www.ibm.com/docs/en/xl-c-and-cpp-aix/16.1.0?topic=ixcod-f, in particular the note that states that this functionality is different from GCC -F (and therefore different from Clang functionality). Our current offering does not support -F (see https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.3?topic=guide-compiler-options) and an error is produced. Note that this is for AIX which is different from Linux on Power. |
…lvm#141614) Remove the redundant -F option from test because not all platforms support -F, which is a g++ extension. The option is unnecessary because all files in the test have paths specified.
Remove the redundant -F option from test because not all platforms support -F, which is a g++ extension. The option is unnecessary because all files in the test have paths specified.