[Driver] Ignore non-clang pch files when -include a.h probes a.h.gch #69204
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of deprecating the "gch probe" as in f726da1, this makes clang ignore files which are not clang pch files (See discussion on PR llvm#67084). This fixes the issues mentioned in the former patch, with GCC-generated .gch files getting in the way when using clang, while maintaining the probing behavior for builds which rely on that.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Hans (zmodem) ChangesInstead of deprecating the "gch probe" as in This fixes the issues mentioned in the former patch, with GCC-generated .gch files getting in the way when using clang, while maintaining the probing behavior for builds which rely on that. Full diff: https://github.com/llvm/llvm-project/pull/69204.diff 8 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9782c123f4c9372..95a671617d72504 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -43,8 +43,8 @@ C/C++ Language Potentially Breaking Changes
- The default extension name for PCH generation (``-c -xc-header`` and ``-c
-xc++-header``) is now ``.pch`` instead of ``.gch``.
-- ``-include a.h`` probing ``a.h.gch`` is deprecated. Change the extension name
- to ``.pch`` or use ``-include-pch a.h.gch``.
+- ``-include a.h`` probing ``a.h.gch`` will now ignore ``a.h.gch`` if it is not
+ a clang pch file or a directory containing any clang pch file.
- Fixed a bug that caused ``__has_cpp_attribute`` and ``__has_c_attribute``
return incorrect values for some C++-11-style attributes. Below is a complete
list of behavior changes.
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 9c00fa50bb95580..370eb4d0fe71e9c 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -441,11 +441,14 @@ def warn_drv_overriding_option : Warning<
def warn_drv_treating_input_as_cxx : Warning<
"treating '%0' input as '%1' when in C++ mode, this behavior is deprecated">,
InGroup<Deprecated>;
-def warn_drv_include_probe_gch : Warning<
- "'%0' probing .gch is deprecated. Use '-include-pch %1' or switch to .pch instead">,
- InGroup<DeprecatedIncludeGch>;
def warn_drv_pch_not_first_include : Warning<
"precompiled header '%0' was ignored because '%1' is not first '-include'">;
+def warn_drv_pch_ignoring_gch_file : Warning<
+ "precompiled header '%0' was ignored because it is not a clang PCH file">,
+ InGroup<IgnoredGCH>;
+def warn_drv_pch_ignoring_gch_dir : Warning<
+ "precompiled header directory '%0' was ignored because it contains no clang PCH files">,
+ InGroup<IgnoredGCH>;
def warn_missing_sysroot : Warning<"no such sysroot directory: '%0'">,
InGroup<DiagGroup<"missing-sysroot">>;
def warn_incompatible_sysroot : Warning<"using sysroot for '%0' but targeting '%1'">,
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 0b09c002191848a..3d82d2955d17896 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -209,7 +209,6 @@ def DeprecatedWritableStr : DiagGroup<"deprecated-writable-strings",
[CXX11CompatDeprecatedWritableStr]>;
def DeprecatedPragma : DiagGroup<"deprecated-pragma">;
def DeprecatedType : DiagGroup<"deprecated-type">;
-def DeprecatedIncludeGch : DiagGroup<"deprecated-include-gch">;
// FIXME: Why is DeprecatedImplementations not in this group?
def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
DeprecatedArrayCompare,
@@ -232,8 +231,7 @@ def Deprecated : DiagGroup<"deprecated", [DeprecatedAnonEnumEnumConversion,
DeprecatedType,
DeprecatedVolatile,
DeprecatedWritableStr,
- DeprecatedRedundantConstexprStaticDef,
- DeprecatedIncludeGch
+ DeprecatedRedundantConstexprStaticDef
]>,
DiagCategory<"Deprecations">;
@@ -443,6 +441,7 @@ def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
def InfiniteRecursion : DiagGroup<"infinite-recursion">;
def PureVirtualCallFromCtorDtor: DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
+def IgnoredGCH : DiagGroup<"ignored-gch">;
def IgnoredReferenceQualifiers : DiagGroup<"ignored-reference-qualifiers">;
def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", [IgnoredReferenceQualifiers]>;
def : DiagGroup<"import">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 94c184435ae14de..677255579e95c27 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1067,6 +1067,35 @@ static void handleAMDGPUCodeObjectVersionOptions(const Driver &D,
}
}
+static bool hasClangPchSignature(const Driver &D, StringRef Path) {
+ if (llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MemBuf =
+ D.getVFS().getBufferForFile(Path))
+ return (*MemBuf)->getBuffer().startswith("CPCH");
+ return false;
+}
+
+static bool gchProbe(const Driver &D, StringRef Path) {
+ llvm::ErrorOr<llvm::vfs::Status> Status = D.getVFS().status(Path);
+ if (!Status)
+ return false;
+
+ if (Status->isDirectory()) {
+ std::error_code EC;
+ for (llvm::vfs::directory_iterator DI = D.getVFS().dir_begin(Path, EC), DE;
+ !EC && DI != DE; DI = DI.increment(EC)) {
+ if (hasClangPchSignature(D, DI->path()))
+ return true;
+ }
+ D.Diag(diag::warn_drv_pch_ignoring_gch_dir) << Path;
+ return false;
+ }
+
+ if (hasClangPchSignature(D, Path))
+ return true;
+ D.Diag(diag::warn_drv_pch_ignoring_gch_file) << Path;
+ return false;
+}
+
void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
const Driver &D, const ArgList &Args,
ArgStringList &CmdArgs,
@@ -1283,11 +1312,9 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
FoundPCH = true;
if (!FoundPCH) {
+ // For GCC compat, probe for a file or directory ending in .gch instead.
llvm::sys::path::replace_extension(P, "gch");
- if (D.getVFS().exists(P)) {
- FoundPCH = true;
- D.Diag(diag::warn_drv_include_probe_gch) << A->getAsString(Args) << P;
- }
+ FoundPCH = gchProbe(D, P.str());
}
if (FoundPCH) {
diff --git a/clang/test/PCH/Inputs/gch-probe.h b/clang/test/PCH/Inputs/gch-probe.h
new file mode 100644
index 000000000000000..940adf12a97ae06
--- /dev/null
+++ b/clang/test/PCH/Inputs/gch-probe.h
@@ -0,0 +1 @@
+void g(void);
diff --git a/clang/test/PCH/Inputs/gch-probe.h.gch b/clang/test/PCH/Inputs/gch-probe.h.gch
new file mode 100644
index 000000000000000..fc551fc058b7c27
--- /dev/null
+++ b/clang/test/PCH/Inputs/gch-probe.h.gch
@@ -0,0 +1 @@
+This is not a pch file.
diff --git a/clang/test/PCH/gch-probe.c b/clang/test/PCH/gch-probe.c
new file mode 100644
index 000000000000000..8b1e1fab5ad97b6
--- /dev/null
+++ b/clang/test/PCH/gch-probe.c
@@ -0,0 +1,9 @@
+// For GCC compatibility, clang should probe also with the .gch extension.
+// RUN: %clang -x c-header -c %s -o %t.h.gch
+// RUN: %clang -fsyntax-only -include %t.h %s
+
+// gch probing should ignore files which are not clang pch files.
+// RUN: %clang -fsyntax-only -include %S/Inputs/gch-probe.h %s 2>&1 | FileCheck %s
+// CHECK: warning: precompiled header '{{.*}}gch-probe.h.gch' was ignored because it is not a clang PCH file
+
+void f(void);
diff --git a/clang/test/PCH/pch-dir.c b/clang/test/PCH/pch-dir.c
index d8ec687b4d72478..fd7d24f9f83ff41 100644
--- a/clang/test/PCH/pch-dir.c
+++ b/clang/test/PCH/pch-dir.c
@@ -23,19 +23,25 @@
// RUN: not %clang_cc1 -include-pch %t.h.gch -DFOO=baz -fsyntax-only %s -print-stats 2> %t.missinglog
// RUN: FileCheck -check-prefix=CHECK-NO-SUITABLE %s < %t.missinglog
+// Don't gch probe directories which contain no pch files.
+// RUN: rm -rf %t.x.h.gch
+// RUN: mkdir -p %t.x.h.gch
+// RUN: not %clang -include %t.x.h -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-IGNORED-DIR %s
+
// CHECK-CBAR: int bar
int FOO;
int get(void) {
#ifdef __cplusplus
- // CHECK-CPP: warning: '-include {{.*}}.h' probing .gch is deprecated. Use '-include-pch {{.*}}.h.gch' or switch to .pch instead
// CHECK-CPP: .h.gch{{[/\\]}}cpp.gch
return i;
#else
- // CHECK-C: warning: '-include {{.*}}.h' probing .gch is deprecated. Use '-include-pch {{.*}}.h.gch' or switch to .pch instead
// CHECK-C: .h.gch{{[/\\]}}c.gch
return j;
#endif
}
// CHECK-NO-SUITABLE: no suitable precompiled header file found in directory
+
+// CHECK-IGNORED-DIR: precompiled header directory '{{.*}}pch-dir.c.tmp.x.h.gch' was ignored because it contains no clang PCH files
+// CHECK-IGNORED-DIR: '{{.*}}pch-dir.c.tmp.x.h' file not found
|
|
Please take a look. This has the benefit of fixing users broken by gcc-generated .gch files without having to wait a release cycle for deprecation, while at the same time not breaking users relying clang performing these probes at all. cc folks from the previous pr: |
zmodem
requested review from
nico,
MaskRay,
tahonermann,
jansvoboda11 and
dwblaikie
tahonermann
approved these changes
Oct 17, 2023
There was a problem hiding this comment.
This looks great to me. Thank you for working on this!
Please give @MaskRay a chance to comment on this before merging it.
|
@MaskRay ping? |
MaskRay
approved these changes
Oct 24, 2023
|
This incorrectly handles |
Bigcheese
added a commit
that referenced
this pull request
Jan 16, 2024
A previous commit (82f75ed) made clang ignore .gch files that were not Clang AST files. This broke `-gmodules`, which embeds the Clang AST into an object file containing debug info. This changes the probing to detect any file format recognized by `llvm::identify_magic()` as potentially containing a Clang AST. Previous PR: #69204
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
A previous commit (82f75ed) made clang ignore .gch files that were not Clang AST files. This broke `-gmodules`, which embeds the Clang AST into an object file containing debug info. This changes the probing to detect any file format recognized by `llvm::identify_magic()` as potentially containing a Clang AST. Previous PR: llvm#69204
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of deprecating the "gch probe" as in
f726da1, this makes clang ignore files which are not clang pch files (See discussion on PR #67084).
This fixes the issues mentioned in the former patch, with GCC-generated .gch files getting in the way when using clang, while maintaining the probing behavior for builds which rely on that.