ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
PrecompiledPreamble hasn't checked the system dependencies changed
before. This result in invalid preamble not being rebuilt if headers
that changed were found in -isystem include paths.
This pattern is sometimes used to avoid showing warnings in third
party code, so we want to correctly handle those cases.
Tested in clangd, see the follow-up patch.
Repository:
rC Clang
https://reviews.llvm.org/D48946
Files:
lib/Frontend/PrecompiledPreamble.cpp
Index: lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
return Overlay;
}
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
/// Keeps a track of files to be deleted in destructor.
class TemporaryFiles {
public:
@@ -311,7 +320,7 @@
Clang->setSourceManager(
new SourceManager(Diagnostics, Clang->getFileManager()));
- auto PreambleDepCollector = std::make_shared<DependencyCollector>();
+ auto PreambleDepCollector = std::make_shared<PreambleDependencyCollector>();
Clang->addDependencyCollector(PreambleDepCollector);
// Remap the main source file to the preamble buffer.
Index: lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
return Overlay;
}
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
/// Keeps a track of files to be deleted in destructor.
class TemporaryFiles {
public:
@@ -311,7 +320,7 @@
Clang->setSourceManager(
new SourceManager(Diagnostics, Clang->getFileManager()));
- auto PreambleDepCollector = std::make_shared<DependencyCollector>();
+ auto PreambleDepCollector = std::make_shared<PreambleDependencyCollector>();
Clang->addDependencyCollector(PreambleDepCollector);
// Remap the main source file to the preamble buffer.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
