diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index 5717816ff99e..bd31c79ba204 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -40,6 +40,7 @@ #include "clang/Basic/SourceLocation.h" #include "llvm/Support/Allocator.h" #include "llvm/Support/DataTypes.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" @@ -508,6 +509,11 @@ public: }; +/// \brief The path used when building modules on demand, which is used +/// to provide a link between the source managers of the different compiler +/// instances. +typedef llvm::ArrayRef > ModuleBuildPath; + /// \brief This class handles loading and caching of source files into memory. /// /// This object owns the MemoryBuffer objects for all of the loaded @@ -645,6 +651,15 @@ class SourceManager : public RefCountedBase { mutable llvm::DenseMap MacroArgsCacheMap; + /// \brief The path of modules being built, which is used to detect + /// cycles in the module dependency graph as modules are being built, as + /// well as to describe + /// + /// There is no way to set this value from the command line. If we ever need + /// to do so (e.g., if on-demand module construction moves out-of-process), + /// we can add a cc1-level option to do so. + SmallVector, 2> StoredModuleBuildPath; + // SourceManager doesn't support copy construction. explicit SourceManager(const SourceManager&) LLVM_DELETED_FUNCTION; void operator=(const SourceManager&) LLVM_DELETED_FUNCTION; @@ -669,6 +684,22 @@ public: /// (likely to change while trying to use them). bool userFilesAreVolatile() const { return UserFilesAreVolatile; } + /// \brief Retrieve the module build path. + ModuleBuildPath getModuleBuildPath() const { + return StoredModuleBuildPath; + } + + /// \brief Set the module build path. + void setModuleBuildPath(ModuleBuildPath path) { + StoredModuleBuildPath.clear(); + StoredModuleBuildPath.append(path.begin(), path.end()); + } + + /// \brief Append an entry to the module build path. + void appendModuleBuildPath(StringRef moduleName, FullSourceLoc importLoc) { + StoredModuleBuildPath.push_back(std::make_pair(moduleName.str(),importLoc)); + } + /// \brief Create the FileID for a memory buffer that will represent the /// FileID for the main source. /// diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 086bb137d46f..6ca8d89ec911 100644 --- a/clang/include/clang/Frontend/DiagnosticRenderer.h +++ b/clang/include/clang/Frontend/DiagnosticRenderer.h @@ -92,7 +92,10 @@ protected: virtual void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, const SourceManager &SM) = 0; - + virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM) = 0; + virtual void beginDiagnostic(DiagOrStoredDiag D, DiagnosticsEngine::Level Level) {} virtual void endDiagnostic(DiagOrStoredDiag D, @@ -103,6 +106,7 @@ private: void emitIncludeStack(SourceLocation Loc, DiagnosticsEngine::Level Level, const SourceManager &SM); void emitIncludeStackRecursively(SourceLocation Loc, const SourceManager &SM); + void emitModuleBuildPath(const SourceManager &SM); void emitMacroExpansionsAndCarets(SourceLocation Loc, DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, @@ -149,7 +153,11 @@ public: virtual void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, const SourceManager &SM); - + + virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM); + virtual void emitNote(SourceLocation Loc, StringRef Message, const SourceManager *SM) = 0; }; diff --git a/clang/include/clang/Frontend/TextDiagnostic.h b/clang/include/clang/Frontend/TextDiagnostic.h index 51f841ddd3c5..859bec2cfe11 100644 --- a/clang/include/clang/Frontend/TextDiagnostic.h +++ b/clang/include/clang/Frontend/TextDiagnostic.h @@ -103,6 +103,10 @@ protected: virtual void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, const SourceManager &SM); + virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM); + private: void emitSnippetAndCaret(SourceLocation Loc, DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h index 857161ddc088..a670267f020c 100644 --- a/clang/include/clang/Lex/PreprocessorOptions.h +++ b/clang/include/clang/Lex/PreprocessorOptions.h @@ -10,6 +10,7 @@ #ifndef LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_ #define LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_ +#include "clang/Basic/SourceLocation.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -120,14 +121,6 @@ public: /// with support for lifetime-qualified pointers. ObjCXXARCStandardLibraryKind ObjCXXARCStandardLibrary; - /// \brief The path of modules being build, which is used to detect - /// cycles in the module dependency graph as modules are being built. - /// - /// There is no way to set this value from the command line. If we ever need - /// to do so (e.g., if on-demand module construction moves out-of-process), - /// we can add a cc1-level option to do so. - SmallVector ModuleBuildPath; - /// \brief Records the set of modules class FailedModulesSet : public llvm::RefCountedBase { llvm::StringSet<> Failed; diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index f87a63542054..150f99578c45 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -762,6 +762,7 @@ static void doCompileMapModule(void *UserData) { /// \brief Compile a module file for the given module, using the options /// provided by the importing compiler instance. static void compileModule(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName) { llvm::LockFileManager Locked(ModuleFileName); @@ -797,10 +798,6 @@ static void compileModule(CompilerInstance &ImportingInstance, // Note the name of the module we're building. Invocation->getLangOpts()->CurrentModule = Module->getTopLevelModuleName(); - // Note that this module is part of the module build path, so that we - // can detect cycles in the module graph. - PPOpts.ModuleBuildPath.push_back(Module->getTopLevelModuleName()); - // Make sure that the failed-module structure has been allocated in // the importing instance, and propagate the pointer to the newly-created // instance. @@ -861,7 +858,18 @@ static void compileModule(CompilerInstance &ImportingInstance, &ImportingInstance.getDiagnosticClient(), /*ShouldOwnClient=*/true, /*ShouldCloneClient=*/true); - + + // Note that this module is part of the module build path, so that we + // can detect cycles in the module graph. + Instance.createFileManager(); // FIXME: Adopt file manager from importer? + Instance.createSourceManager(Instance.getFileManager()); + SourceManager &SourceMgr = Instance.getSourceManager(); + SourceMgr.setModuleBuildPath( + ImportingInstance.getSourceManager().getModuleBuildPath()); + SourceMgr.appendModuleBuildPath(Module->getTopLevelModuleName(), + FullSourceLoc(ImportLoc, ImportingInstance.getSourceManager())); + + // Construct a module-generating action. GenerateModuleAction CreateModuleAction; @@ -939,14 +947,17 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, // build the module. // Check whether there is a cycle in the module graph. - SmallVectorImpl &ModuleBuildPath - = getPreprocessorOpts().ModuleBuildPath; - SmallVectorImpl::iterator Pos - = std::find(ModuleBuildPath.begin(), ModuleBuildPath.end(), ModuleName); - if (Pos != ModuleBuildPath.end()) { + ModuleBuildPath Path = getSourceManager().getModuleBuildPath(); + ModuleBuildPath::iterator Pos = Path.begin(), PosEnd = Path.end(); + for (; Pos != PosEnd; ++Pos) { + if (Pos->first == ModuleName) + break; + } + + if (Pos != PosEnd) { SmallString<256> CyclePath; - for (; Pos != ModuleBuildPath.end(); ++Pos) { - CyclePath += *Pos; + for (; Pos != PosEnd; ++Pos) { + CyclePath += Pos->first; CyclePath += " -> "; } CyclePath += ModuleName; @@ -970,7 +981,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, getDiagnostics().Report(ModuleNameLoc, diag::warn_module_build) << ModuleName; BuildingModule = true; - compileModule(*this, Module, ModuleFileName); + compileModule(*this, ModuleNameLoc, Module, ModuleFileName); ModuleFile = FileMgr->getFile(ModuleFileName); if (!ModuleFile) @@ -1040,7 +1051,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, return ModuleLoadResult(); } - compileModule(*this, Module, ModuleFileName); + compileModule(*this, ModuleNameLoc, Module, ModuleFileName); // Try loading the module again. ModuleFile = FileMgr->getFile(ModuleFileName); diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index 3143cc7b324a..3599df82c79f 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -205,8 +205,10 @@ void DiagnosticRenderer::emitIncludeStack(SourceLocation Loc, /// on the way back down. void DiagnosticRenderer::emitIncludeStackRecursively(SourceLocation Loc, const SourceManager &SM) { - if (Loc.isInvalid()) + if (Loc.isInvalid()) { + emitModuleBuildPath(SM); return; + } PresumedLoc PLoc = SM.getPresumedLoc(Loc, DiagOpts->ShowPresumedLoc); if (PLoc.isInvalid()) @@ -219,6 +221,21 @@ void DiagnosticRenderer::emitIncludeStackRecursively(SourceLocation Loc, emitIncludeLocation(Loc, PLoc, SM); } +/// \brief Emit the module build path, for cases where a module is (re-)built +/// on demand. +void DiagnosticRenderer::emitModuleBuildPath(const SourceManager &SM) { + ModuleBuildPath Path = SM.getModuleBuildPath(); + for (unsigned I = 0, N = Path.size(); I != N; ++I) { + const SourceManager &CurSM = Path[I].second.getManager(); + SourceLocation CurLoc = Path[I].second; + emitBuildingModuleLocation(CurLoc, + CurSM.getPresumedLoc(CurLoc, + DiagOpts->ShowPresumedLoc), + Path[I].first, + CurSM); + } +} + // Helper function to fix up source ranges. It takes in an array of ranges, // and outputs an array of ranges where we want to draw the range highlighting // around the location specified by CaretLoc. @@ -390,6 +407,20 @@ void DiagnosticNoteRenderer::emitIncludeLocation(SourceLocation Loc, emitNote(Loc, Message.str(), &SM); } +void +DiagnosticNoteRenderer::emitBuildingModuleLocation(SourceLocation Loc, + PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM) { + // Generate a note indicating the include location. + SmallString<200> MessageStorage; + llvm::raw_svector_ostream Message(MessageStorage); + Message << "while building module '" << ModuleName << "' imported from " + << PLoc.getFilename() << ':' << PLoc.getLine() << ":"; + emitNote(Loc, Message.str(), &SM); +} + + void DiagnosticNoteRenderer::emitBasicNote(StringRef Message) { emitNote(SourceLocation(), Message, 0); } diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index 35dabad60657..65df875aea38 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -884,6 +884,17 @@ void TextDiagnostic::emitIncludeLocation(SourceLocation Loc, OS << "In included file:\n"; } +void TextDiagnostic::emitBuildingModuleLocation(SourceLocation Loc, + PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM) { + if (DiagOpts->ShowLocation) + OS << "While building module '" << ModuleName << "' imported from " + << PLoc.getFilename() << ':' << PLoc.getLine() << ":\n"; + else + OS << "While building module '" << ModuleName << "':\n"; +} + /// \brief Emit a code snippet and caret line. /// /// This routine emits a single line's code snippet and caret line.. diff --git a/clang/test/Modules/build-fail-notes.m b/clang/test/Modules/build-fail-notes.m new file mode 100644 index 000000000000..fe5522342fec --- /dev/null +++ b/clang/test/Modules/build-fail-notes.m @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodule-cache-path %t -fmodules -F %S/Inputs -DgetModuleVersion="epic fail" %s 2>&1 | FileCheck %s + +@__experimental_modules_import DependsOnModule; + +// CHECK: While building module 'DependsOnModule' imported from +// CHECK: While building module 'Module' imported from +// CHECK: error: expected ';' after top level declarator +// CHECK: note: expanded from macro 'getModuleVersion' +// CHECK: fatal error: could not build module 'Module' +// CHECK: fatal error: could not build module 'DependsOnModule' +// CHECK-NOT: error: diff --git a/clang/test/Modules/cycles.c b/clang/test/Modules/cycles.c index 256f118cc025..5fcb41ec1f64 100644 --- a/clang/test/Modules/cycles.c +++ b/clang/test/Modules/cycles.c @@ -3,10 +3,11 @@ // FIXME: When we have a syntax for modules in C, use that. @__experimental_modules_import MutuallyRecursive1; -// FIXME: Lots of redundant diagnostics here, because the preprocessor -// can't currently tell the parser not to try to load the module again. - +// CHECK: While building module 'MutuallyRecursive1' imported from +// CHECK: While building module 'MutuallyRecursive2' imported from // CHECK: MutuallyRecursive2.h:3:32: fatal error: cyclic dependency in module 'MutuallyRecursive1': MutuallyRecursive1 -> MutuallyRecursive2 -> MutuallyRecursive1 +// CHECK: While building module 'MutuallyRecursive1' imported from // CHECK: MutuallyRecursive1.h:2:32: fatal error: could not build module 'MutuallyRecursive2' // CHECK: cycles.c:4:32: fatal error: could not build module 'MutuallyRecursive1' +// CHECK-NOT: error: diff --git a/clang/test/Modules/epic-fail.m b/clang/test/Modules/epic-fail.m index 7d2cdcf602cb..32ac52d89ad1 100644 --- a/clang/test/Modules/epic-fail.m +++ b/clang/test/Modules/epic-fail.m @@ -4,8 +4,10 @@ @__experimental_modules_import Module; @__experimental_modules_import DependsOnModule; +// CHECK: While building module 'Module' imported from // CHECK: error: expected ';' after top level declarator // CHECK: note: expanded from macro 'getModuleVersion' // CHECK: fatal error: could not build module 'Module' +// CHECK: While building module 'DependsOnModule' imported from // CHECK: fatal error: could not build module 'Module' // CHECK-NOT: error: