<div dir="ltr"><div>LibclangReparseTest.ReparseWithModule fails for me. (With `./configure --enable-optimized --enable-targets=host-only --enable-libedit=no --disable-threads --disable-pthreads --without-llvmgcc --without-llvmgxx`)</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 30, 2014 at 1:04 PM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: benlangmuir<br>
Date: Mon Jun 30 15:04:14 2014<br>
New Revision: 212060<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=212060&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=212060&view=rev</a><br>
Log:<br>
Consider module depedencies when checking a preamble in libclang<br>
<br>
Add module dependencies (header files, module map files) to the list of<br>
files to check when deciding whether to rebuild a preamble. That fixes<br>
using preambles with module imports so long as they are in<br>
non-overridden files.<br>
<br>
My intent is to use to unify the existing dependency collectors to the<br>
new “DependencyCollectory” interface from this commit, starting with the<br>
DependencyFileGenerator.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Frontend/CompilerInstance.h<br>
cfe/trunk/include/clang/Frontend/Utils.h<br>
cfe/trunk/lib/Frontend/ASTUnit.cpp<br>
cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
cfe/trunk/lib/Frontend/DependencyFile.cpp<br>
cfe/trunk/unittests/libclang/LibclangTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Mon Jun 30 15:04:14 2014<br>
@@ -111,6 +111,8 @@ class CompilerInstance : public ModuleLo<br>
/// \brief The dependency file generator.<br>
std::unique_ptr<DependencyFileGenerator> TheDependencyFileGenerator;<br>
<br>
+ std::vector<std::shared_ptr<DependencyCollector>> DependencyCollectors;<br>
+<br>
/// \brief The set of top-level modules that has already been loaded,<br>
/// along with the module map<br>
llvm::DenseMap<const IdentifierInfo *, Module *> KnownModules;<br>
@@ -711,6 +713,10 @@ public:<br>
GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc) override;<br>
<br>
bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override;<br>
+<br>
+ void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {<br>
+ DependencyCollectors.push_back(std::move(Listener));<br>
+ }<br>
};<br>
<br>
} // end namespace clang<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/Utils.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/Utils.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/Utils.h Mon Jun 30 15:04:14 2014<br>
@@ -69,6 +69,39 @@ void InitializePreprocessor(Preprocessor<br>
void DoPrintPreprocessedInput(Preprocessor &PP, raw_ostream* OS,<br>
const PreprocessorOutputOptions &Opts);<br>
<br>
+/// An interface for collecting the dependencies of a compilation. Users should<br>
+/// use \c attachToPreprocessor and \c attachToASTReader to get all of the<br>
+/// dependencies.<br>
+// FIXME: Migrate DependencyFileGen, DependencyGraphGen, ModuleDepCollectory to<br>
+// use this interface.<br>
+class DependencyCollector {<br>
+public:<br>
+ void attachToPreprocessor(Preprocessor &PP);<br>
+ void attachToASTReader(ASTReader &R);<br>
+ llvm::ArrayRef<std::string> getDependencies() const { return Dependencies; }<br>
+<br>
+ /// Called when a new file is seen. Return true if \p Filename should be added<br>
+ /// to the list of dependencies.<br>
+ ///<br>
+ /// The default implementation ignores <built-in> and system files.<br>
+ virtual bool sawDependency(StringRef Filename, bool FromModule,<br>
+ bool IsSystem, bool IsModuleFile, bool IsMissing);<br>
+ /// Called when the end of the main file is reached.<br>
+ virtual void finishedMainFile() { }<br>
+ /// Return true if system files should be passed to sawDependency().<br>
+ virtual bool needSystemDependencies() { return false; }<br>
+ virtual ~DependencyCollector();<br>
+<br>
+public: // implementation detail<br>
+ /// Add a dependency \p Filename if it has not been seen before and<br>
+ /// sawDependency() returns true.<br>
+ void maybeAddDependency(StringRef Filename, bool FromModule, bool IsSystem,<br>
+ bool IsModuleFile, bool IsMissing);<br>
+private:<br>
+ llvm::StringSet<> Seen;<br>
+ std::vector<std::string> Dependencies;<br>
+};<br>
+<br>
/// Builds a depdenency file when attached to a Preprocessor (for includes) and<br>
/// ASTReader (for module imports), and writes it out at the end of processing<br>
/// a source file. Users should attach to the ast reader whenever a module is<br>
<br>
Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Mon Jun 30 15:04:14 2014<br>
@@ -1609,6 +1609,9 @@ llvm::MemoryBuffer *ASTUnit::getMainBuff<br>
Clang->setSourceManager(new SourceManager(getDiagnostics(),<br>
Clang->getFileManager()));<br>
<br>
+ auto PreambleDepCollector = std::make_shared<DependencyCollector>();<br>
+ Clang->addDependencyCollector(PreambleDepCollector);<br>
+<br>
std::unique_ptr<PrecompilePreambleAction> Act;<br>
Act.reset(new PrecompilePreambleAction(*this));<br>
if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0])) {<br>
@@ -1657,29 +1660,20 @@ llvm::MemoryBuffer *ASTUnit::getMainBuff<br>
// so we can verify whether they have changed or not.<br>
FilesInPreamble.clear();<br>
SourceManager &SourceMgr = Clang->getSourceManager();<br>
- const llvm::MemoryBuffer *MainFileBuffer<br>
- = SourceMgr.getBuffer(SourceMgr.getMainFileID());<br>
- for (SourceManager::fileinfo_iterator F = SourceMgr.fileinfo_begin(),<br>
- FEnd = SourceMgr.fileinfo_end();<br>
- F != FEnd;<br>
- ++F) {<br>
- const FileEntry *File = F->second->OrigEntry;<br>
- if (!File)<br>
- continue;<br>
- const llvm::MemoryBuffer *Buffer = F->second->getRawBuffer();<br>
- if (Buffer == MainFileBuffer)<br>
+ for (auto &Filename : PreambleDepCollector->getDependencies()) {<br>
+ const FileEntry *File = Clang->getFileManager().getFile(Filename);<br>
+ if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))<br>
continue;<br>
-<br>
if (time_t ModTime = File->getModificationTime()) {<br>
FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(<br>
- F->second->getSize(), ModTime);<br>
+ File->getSize(), ModTime);<br>
} else {<br>
- assert(F->second->getSize() == Buffer->getBufferSize());<br>
+ llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);<br>
FilesInPreamble[File->getName()] =<br>
PreambleFileHash::createForMemoryBuffer(Buffer);<br>
}<br>
}<br>
-<br>
+<br>
PreambleRebuildCounter = 1;<br>
PreprocessorOpts.eraseRemappedFile(<br>
PreprocessorOpts.remapped_file_buffer_end() - 1);<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 30 15:04:14 2014<br>
@@ -288,6 +288,9 @@ void CompilerInstance::createPreprocesso<br>
AttachDependencyGraphGen(*PP, DepOpts.DOTOutputFile,<br>
getHeaderSearchOpts().Sysroot);<br>
<br>
+ for (auto &Listener : DependencyCollectors)<br>
+ Listener->attachToPreprocessor(*PP);<br>
+<br>
// If we don't have a collector, but we are collecting module dependencies,<br>
// then we're the top level compiler instance and need to create one.<br>
if (!ModuleDepCollector && !DepOpts.ModuleDependencyOutputDir.empty())<br>
@@ -1233,6 +1236,9 @@ CompilerInstance::loadModule(SourceLocat<br>
if (ModuleDepCollector)<br>
ModuleDepCollector->attachToASTReader(*ModuleManager);<br>
<br>
+ for (auto &Listener : DependencyCollectors)<br>
+ Listener->attachToASTReader(*ModuleManager);<br>
+<br>
// Try to load the module file.<br>
unsigned ARRFlags = ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing;<br>
switch (ModuleManager->ReadAST(ModuleFileName, serialization::MK_Module,<br>
<br>
Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/DependencyFile.cpp Mon Jun 30 15:04:14 2014<br>
@@ -29,6 +29,105 @@<br>
using namespace clang;<br>
<br>
namespace {<br>
+struct DepCollectorPPCallbacks : public PPCallbacks {<br>
+ DependencyCollector &DepCollector;<br>
+ SourceManager &SM;<br>
+ DepCollectorPPCallbacks(DependencyCollector &L, SourceManager &SM)<br>
+ : DepCollector(L), SM(SM) { }<br>
+<br>
+ void FileChanged(SourceLocation Loc, FileChangeReason Reason,<br>
+ SrcMgr::CharacteristicKind FileType,<br>
+ FileID PrevFID) override {<br>
+ if (Reason != PPCallbacks::EnterFile)<br>
+ return;<br>
+<br>
+ // Dependency generation really does want to go all the way to the<br>
+ // file entry for a source location to find out what is depended on.<br>
+ // We do not want #line markers to affect dependency generation!<br>
+ const FileEntry *FE =<br>
+ SM.getFileEntryForID(SM.getFileID(SM.getExpansionLoc(Loc)));<br>
+ if (!FE)<br>
+ return;<br>
+<br>
+ StringRef Filename = FE->getName();<br>
+<br>
+ // Remove leading "./" (or ".//" or "././" etc.)<br>
+ while (Filename.size() > 2 && Filename[0] == '.' &&<br>
+ llvm::sys::path::is_separator(Filename[1])) {<br>
+ Filename = Filename.substr(1);<br>
+ while (llvm::sys::path::is_separator(Filename[0]))<br>
+ Filename = Filename.substr(1);<br>
+ }<br>
+<br>
+ DepCollector.maybeAddDependency(Filename, /*FromModule*/false,<br>
+ FileType != SrcMgr::C_User,<br>
+ /*IsModuleFile*/false, /*IsMissing*/false);<br>
+ }<br>
+<br>
+ void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,<br>
+ StringRef FileName, bool IsAngled,<br>
+ CharSourceRange FilenameRange, const FileEntry *File,<br>
+ StringRef SearchPath, StringRef RelativePath,<br>
+ const Module *Imported) override {<br>
+ if (!File)<br>
+ DepCollector.maybeAddDependency(FileName, /*FromModule*/false,<br>
+ /*IsSystem*/false, /*IsModuleFile*/false,<br>
+ /*IsMissing*/true);<br>
+ // Files that actually exist are handled by FileChanged.<br>
+ }<br>
+<br>
+ void EndOfMainFile() override {<br>
+ DepCollector.finishedMainFile();<br>
+ }<br>
+};<br>
+<br>
+struct DepCollectorASTListener : public ASTReaderListener {<br>
+ DependencyCollector &DepCollector;<br>
+ DepCollectorASTListener(DependencyCollector &L) : DepCollector(L) { }<br>
+ bool needsInputFileVisitation() override { return true; }<br>
+ bool needsSystemInputFileVisitation() override {<br>
+ return DepCollector.needSystemDependencies();<br>
+ }<br>
+ void visitModuleFile(StringRef Filename) override {<br>
+ DepCollector.maybeAddDependency(Filename, /*FromModule*/true,<br>
+ /*IsSystem*/false, /*IsModuleFile*/true,<br>
+ /*IsMissing*/false);<br>
+ }<br>
+ bool visitInputFile(StringRef Filename, bool IsSystem,<br>
+ bool IsOverridden) override {<br>
+ if (IsOverridden)<br>
+ return true;<br>
+<br>
+ DepCollector.maybeAddDependency(Filename, /*FromModule*/true, IsSystem,<br>
+ /*IsModuleFile*/false, /*IsMissing*/false);<br>
+ return true;<br>
+ }<br>
+};<br>
+} // end anonymous namespace<br>
+<br>
+void DependencyCollector::maybeAddDependency(StringRef Filename, bool FromModule,<br>
+ bool IsSystem, bool IsModuleFile,<br>
+ bool IsMissing) {<br>
+ if (Seen.insert(Filename) &&<br>
+ sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))<br>
+ Dependencies.push_back(Filename);<br>
+}<br>
+<br>
+bool DependencyCollector::sawDependency(StringRef Filename, bool FromModule,<br>
+ bool IsSystem, bool IsModuleFile,<br>
+ bool IsMissing) {<br>
+ return Filename != "<built-in>" && (needSystemDependencies() || !IsSystem);<br>
+}<br>
+<br>
+DependencyCollector::~DependencyCollector() { }<br>
+void DependencyCollector::attachToPreprocessor(Preprocessor &PP) {<br>
+ PP.addPPCallbacks(new DepCollectorPPCallbacks(*this, PP.getSourceManager()));<br>
+}<br>
+void DependencyCollector::attachToASTReader(ASTReader &R) {<br>
+ R.addListener(new DepCollectorASTListener(*this));<br>
+}<br>
+<br>
+namespace {<br>
/// Private implementation for DependencyFileGenerator<br>
class DFGImpl : public PPCallbacks {<br>
std::vector<std::string> Files;<br>
<br>
Modified: cfe/trunk/unittests/libclang/LibclangTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/LibclangTest.cpp?rev=212060&r1=212059&r2=212060&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/libclang/LibclangTest.cpp?rev=212060&r1=212059&r2=212060&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/libclang/LibclangTest.cpp (original)<br>
+++ cfe/trunk/unittests/libclang/LibclangTest.cpp Mon Jun 30 15:04:14 2014<br>
@@ -340,9 +340,9 @@ TEST(libclang, ModuleMapDescriptor) {<br>
}<br>
<br>
class LibclangReparseTest : public ::testing::Test {<br>
- std::string TestDir;<br>
std::set<std::string> Files;<br>
public:<br>
+ std::string TestDir;<br>
CXIndex Index;<br>
CXTranslationUnit ClangTU;<br>
unsigned TUFlags;<br>
@@ -407,6 +407,39 @@ TEST_F(LibclangReparseTest, Reparse) {<br>
nullptr, 0, TUFlags);<br>
EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU));<br>
DisplayDiagnostics();<br>
+<br>
+ // Immedaitely reparse.<br>
+ ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */));<br>
+ EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU));<br>
+<br>
+ std::string NewHeaderContents =<br>
+ std::string(HeaderTop) + "int baz;" + HeaderBottom;<br>
+ WriteFile(HeaderName, NewHeaderContents);<br>
+<br>
+ // Reparse after fix.<br>
+ ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */));<br>
+ EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));<br>
+}<br>
+<br>
+TEST_F(LibclangReparseTest, ReparseWithModule) {<br>
+ const char *HeaderTop = "#ifndef H\n#define H\nstruct Foo { int bar;";<br>
+ const char *HeaderBottom = "\n};\n#endif\n";<br>
+ const char *MFile = "#include \"HeaderFile.h\"\nint main() {"<br>
+ " struct Foo foo; foo.bar = 7; foo.baz = 8; }\n";<br>
+ const char *ModFile = "module A { header \"HeaderFile.h\" }\n";<br>
+ std::string HeaderName = "HeaderFile.h";<br>
+ std::string MName = "MFile.m";<br>
+ std::string ModName = "module.modulemap";<br>
+ WriteFile(MName, MFile);<br>
+ WriteFile(HeaderName, std::string(HeaderTop) + HeaderBottom);<br>
+ WriteFile(ModName, ModFile);<br>
+<br>
+ const char *Args[] = { "-fmodules", "-I", TestDir.c_str() };<br>
+ int NumArgs = sizeof(Args) / sizeof(Args[0]);<br>
+ ClangTU = clang_parseTranslationUnit(Index, MName.c_str(), Args, NumArgs,<br>
+ nullptr, 0, TUFlags);<br>
+ EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU));<br>
+ DisplayDiagnostics();<br>
<br>
// Immedaitely reparse.<br>
ASSERT_TRUE(ReparseTU(0, nullptr /* No unsaved files. */));<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>