diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index bd31c79ba204..d79cbfe5ae8a 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -430,6 +430,11 @@ public: /// \returns true if an error occurred that prevented the source-location /// entry from being loaded. virtual bool ReadSLocEntry(int ID) = 0; + + /// \brief Retrieve the module import location and name for the given ID, if + /// in fact it was loaded from a module (rather than, say, a precompiled + /// header). + virtual std::pair getModuleImportLoc(int ID) = 0; }; @@ -990,6 +995,21 @@ public: return Entry.getFile().getIncludeLoc(); } + // \brief Returns the import location if the given source location is + // located within a module, or an invalid location if the source location + // is within the current translation unit. + std::pair + getModuleImportLoc(SourceLocation Loc) const { + FileID FID = getFileID(Loc); + + // Positive file IDs are in the current translation unit, and -1 is a + // placeholder. + if (FID.ID >= -1) + return std::make_pair(SourceLocation(), ""); + + return ExternalSLocEntries->getModuleImportLoc(FID.ID); + } + /// \brief Given a SourceLocation object \p Loc, return the expansion /// location referenced by the ID. SourceLocation getExpansionLoc(SourceLocation Loc) const { diff --git a/clang/include/clang/Frontend/DiagnosticRenderer.h b/clang/include/clang/Frontend/DiagnosticRenderer.h index 6ca8d89ec911..f46eb5c50069 100644 --- a/clang/include/clang/Frontend/DiagnosticRenderer.h +++ b/clang/include/clang/Frontend/DiagnosticRenderer.h @@ -19,6 +19,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/PointerUnion.h" namespace clang { @@ -92,6 +93,9 @@ protected: virtual void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, const SourceManager &SM) = 0; + virtual void emitImportLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM) = 0; virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, StringRef ModuleName, const SourceManager &SM) = 0; @@ -103,9 +107,12 @@ protected: private: - void emitIncludeStack(SourceLocation Loc, DiagnosticsEngine::Level Level, - const SourceManager &SM); + void emitIncludeStack(SourceLocation Loc, PresumedLoc PLoc, + DiagnosticsEngine::Level Level, const SourceManager &SM); void emitIncludeStackRecursively(SourceLocation Loc, const SourceManager &SM); + void emitImportStack(SourceLocation Loc, const SourceManager &SM); + void emitImportStackRecursively(SourceLocation Loc, StringRef ModuleName, + const SourceManager &SM); void emitModuleBuildPath(const SourceManager &SM); void emitMacroExpansionsAndCarets(SourceLocation Loc, DiagnosticsEngine::Level Level, @@ -154,6 +161,10 @@ public: PresumedLoc PLoc, const SourceManager &SM); + virtual void emitImportLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM); + virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, StringRef ModuleName, const SourceManager &SM); diff --git a/clang/include/clang/Frontend/TextDiagnostic.h b/clang/include/clang/Frontend/TextDiagnostic.h index 859bec2cfe11..bce4e05efc39 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 emitImportLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM); + virtual void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, StringRef ModuleName, const SourceManager &SM); diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 6b3c4f107f24..d51917fcb19d 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1546,6 +1546,10 @@ public: /// \brief Read the source location entry with index ID. virtual bool ReadSLocEntry(int ID); + /// \brief Retrieve the module import location and module name for the + /// given source manager entry ID. + virtual std::pair getModuleImportLoc(int ID); + /// \brief Retrieve the global submodule ID given a module and its local ID /// number. serialization::SubmoduleID diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index 3599df82c79f..4a332ae82fac 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -132,7 +132,7 @@ void DiagnosticRenderer::emitDiagnostic(SourceLocation Loc, // First, if this diagnostic is not in the main file, print out the // "included from" lines. - emitIncludeStack(PLoc.getIncludeLoc(), Level, *SM); + emitIncludeStack(Loc, PLoc, Level, *SM); } // Next, emit the actual diagnostic message. @@ -184,21 +184,30 @@ void DiagnosticRenderer::emitStoredDiagnostic(StoredDiagnostic &Diag) { /// repeated warnings occur within the same file. It also handles the logic /// of customizing the formatting and display of the include stack. /// +/// \param Loc The diagnostic location. +/// \param PLoc The presumed location of the diagnostic location. /// \param Level The diagnostic level of the message this stack pertains to. -/// \param Loc The include location of the current file (not the diagnostic -/// location). void DiagnosticRenderer::emitIncludeStack(SourceLocation Loc, + PresumedLoc PLoc, DiagnosticsEngine::Level Level, const SourceManager &SM) { + SourceLocation IncludeLoc = PLoc.getIncludeLoc(); + // Skip redundant include stacks altogether. - if (LastIncludeLoc == Loc) + if (LastIncludeLoc == IncludeLoc) return; - LastIncludeLoc = Loc; + + LastIncludeLoc = IncludeLoc; if (!DiagOpts->ShowNoteIncludeStack && Level == DiagnosticsEngine::Note) return; - - emitIncludeStackRecursively(Loc, SM); + + if (IncludeLoc.isValid()) + emitIncludeStackRecursively(IncludeLoc, SM); + else { + emitModuleBuildPath(SM); + emitImportStack(Loc, SM); + } } /// \brief Helper to recursivly walk up the include stack and print each layer @@ -213,7 +222,17 @@ void DiagnosticRenderer::emitIncludeStackRecursively(SourceLocation Loc, PresumedLoc PLoc = SM.getPresumedLoc(Loc, DiagOpts->ShowPresumedLoc); if (PLoc.isInvalid()) return; - + + // If this source location was imported from a module, print the module + // import stack rather than the + // FIXME: We want submodule granularity here. + std::pair Imported = SM.getModuleImportLoc(Loc); + if (Imported.first.isValid()) { + // This location was imported by a module. Emit the module import stack. + emitImportStackRecursively(Imported.first, Imported.second, SM); + return; + } + // Emit the other include frames first. emitIncludeStackRecursively(PLoc.getIncludeLoc(), SM); @@ -221,6 +240,41 @@ void DiagnosticRenderer::emitIncludeStackRecursively(SourceLocation Loc, emitIncludeLocation(Loc, PLoc, SM); } +/// \brief Emit the module import stack associated with the current location. +void DiagnosticRenderer::emitImportStack(SourceLocation Loc, + const SourceManager &SM) { + if (Loc.isInvalid()) { + emitModuleBuildPath(SM); + return; + } + + std::pair NextImportLoc + = SM.getModuleImportLoc(Loc); + emitImportStackRecursively(NextImportLoc.first, NextImportLoc.second, SM); +} + +/// \brief Helper to recursivly walk up the import stack and print each layer +/// on the way back down. +void DiagnosticRenderer::emitImportStackRecursively(SourceLocation Loc, + StringRef ModuleName, + const SourceManager &SM) { + if (Loc.isInvalid()) { + return; + } + + PresumedLoc PLoc = SM.getPresumedLoc(Loc, DiagOpts->ShowPresumedLoc); + if (PLoc.isInvalid()) + return; + + // Emit the other import frames first. + std::pair NextImportLoc + = SM.getModuleImportLoc(Loc); + emitImportStackRecursively(NextImportLoc.first, NextImportLoc.second, SM); + + // Emit the inclusion text/note. + emitImportLocation(Loc, PLoc, ModuleName, SM); +} + /// \brief Emit the module build path, for cases where a module is (re-)built /// on demand. void DiagnosticRenderer::emitModuleBuildPath(const SourceManager &SM) { @@ -407,6 +461,18 @@ void DiagnosticNoteRenderer::emitIncludeLocation(SourceLocation Loc, emitNote(Loc, Message.str(), &SM); } +void DiagnosticNoteRenderer::emitImportLocation(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 << "in module '" << ModuleName << "' imported from " + << PLoc.getFilename() << ':' << PLoc.getLine() << ":"; + emitNote(Loc, Message.str(), &SM); +} + void DiagnosticNoteRenderer::emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index 65df875aea38..fe82229c919b 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -884,6 +884,16 @@ void TextDiagnostic::emitIncludeLocation(SourceLocation Loc, OS << "In included file:\n"; } +void TextDiagnostic::emitImportLocation(SourceLocation Loc, PresumedLoc PLoc, + StringRef ModuleName, + const SourceManager &SM) { + if (DiagOpts->ShowLocation) + OS << "In module '" << ModuleName << "' imported from " + << PLoc.getFilename() << ':' << PLoc.getLine() << ":\n"; + else + OS << "In module " << ModuleName << "':\n"; +} + void TextDiagnostic::emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc, StringRef ModuleName, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index c1428d5c68bf..908d8c573c05 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -1000,6 +1000,25 @@ bool ASTReader::ReadSLocEntry(int ID) { return false; } +std::pair ASTReader::getModuleImportLoc(int ID) { + if (ID == 0) + return std::make_pair(SourceLocation(), ""); + + if (unsigned(-ID) - 2 >= getTotalNumSLocs() || ID > 0) { + Error("source location entry ID out-of-range for AST file"); + return std::make_pair(SourceLocation(), ""); + } + + // Find which module file this entry lands in. + ModuleFile *M = GlobalSLocEntryMap.find(-ID)->second; + if (M->Kind != MK_Module) + return std::make_pair(SourceLocation(), ""); + + // FIXME: Can we map this down to a particular submodule? That would be + // ideal. + return std::make_pair(M->ImportLoc, llvm::sys::path::stem(M->FileName)); +} + /// \brief Find the location where the module F is imported. SourceLocation ASTReader::getImportLocation(ModuleFile *F) { if (F->ImportLoc.isValid()) diff --git a/clang/test/Modules/build-fail-notes.m b/clang/test/Modules/build-fail-notes.m index fe5522342fec..4c5d5f5de524 100644 --- a/clang/test/Modules/build-fail-notes.m +++ b/clang/test/Modules/build-fail-notes.m @@ -10,3 +10,10 @@ // CHECK: fatal error: could not build module 'Module' // CHECK: fatal error: could not build module 'DependsOnModule' // CHECK-NOT: error: + +// RUN: %clang_cc1 -fmodule-cache-path %t -fmodules -F %S/Inputs %s -fdiagnostics-show-note-include-stack 2>&1 | FileCheck -check-prefix=CHECK-REDEF %s +extern int Module; + +// CHECK-REDEF: In module 'DependsOnModule' imported from +// CHECK-REDEF: In module 'Module' imported from +// CHECK-REDEF: Module.h:15:12: note: previous definition is here