diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -135,8 +135,7 @@ : SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback), RelationsCallback(RelationsCallback), IncludeGraphCallback(IncludeGraphCallback), Collector(C), - Includes(std::move(Includes)), Opts(Opts), - PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) { + Includes(std::move(Includes)), Opts(Opts) { this->Opts.ShouldTraverseDecl = [this](const Decl *D) { // Many operations performed during indexing is linear in terms of depth // of the decl (USR generation, name lookups, figuring out role of a @@ -154,7 +153,6 @@ std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { - CI.getPreprocessor().addCommentHandler(PragmaHandler.get()); Includes->addSystemHeadersMapping(CI.getLangOpts()); if (IncludeGraphCallback != nullptr) CI.getPreprocessor().addPPCallbacks( @@ -203,7 +201,6 @@ std::shared_ptr Collector; std::unique_ptr Includes; index::IndexingOptions Opts; - std::unique_ptr PragmaHandler; IncludeGraph IG; }; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -9,6 +9,8 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLCOLLECTOR_H #include "CollectMacros.h" +#include "clang-include-cleaner/Record.h" +#include "clang-include-cleaner/Types.h" #include "index/CanonicalIncludes.h" #include "index/Ref.h" #include "index/Relation.h" @@ -24,6 +26,7 @@ #include "clang/Sema/CodeCompleteConsumer.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" #include #include @@ -123,8 +126,12 @@ void setPreprocessor(std::shared_ptr PP) override { this->PP = PP.get(); + PI.record(*this->PP); + } + void setPreprocessor(Preprocessor &PP) { + this->PP = &PP; + PI.record(*this->PP); } - void setPreprocessor(Preprocessor &PP) { this->PP = &PP; } bool handleDeclOccurrence(const Decl *D, index::SymbolRoleSet Roles, @@ -166,13 +173,24 @@ // All Symbols collected from the AST. SymbolSlab::Builder Symbols; - // File IDs for Symbol.IncludeHeaders. - // The final spelling is calculated in finish(). + // File IDs to distinguish imports from includes. llvm::DenseMap IncludeFiles; + void setIncludeLocation(const Symbol &S, SourceLocation); + + // Providers for Symbol.IncludeHeaders. + // The final spelling is calculated in finish(). + llvm::DenseMap> + SymbolProviders; + // Mapping from symbol ID to final include spelling. + // Needs to be persisted here because Symbols don't store any of their data. + llvm::DenseMap SymbolIncludeSpelling; // Files which contain ObjC symbols. // This is finalized and used in finish(). llvm::DenseSet FilesWithObjCConstructs; - void setIncludeLocation(const Symbol &S, SourceLocation); + + void + setSymbolProviders(const Symbol &S, + const llvm::SmallVector &Headers); // Indexed macros, to be erased if they turned out to be include guards. llvm::DenseSet IndexedMacros; // All refs collected from the AST. It includes: @@ -184,6 +202,7 @@ RelationSlab::Builder Relations; ASTContext *ASTCtx; Preprocessor *PP = nullptr; + include_cleaner::PragmaIncludes PI; std::shared_ptr CompletionAllocator; std::unique_ptr CompletionTUInfo; Options Opts; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -13,8 +13,12 @@ #include "ExpectedTypes.h" #include "SourceCode.h" #include "URI.h" +#include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Record.h" +#include "clang-include-cleaner/Types.h" #include "index/CanonicalIncludes.h" #include "index/Relation.h" +#include "index/Symbol.h" #include "index/SymbolID.h" #include "index/SymbolLocation.h" #include "clang/AST/Decl.h" @@ -30,10 +34,15 @@ #include "clang/Lex/Token.h" #include "clang/Tooling/Inclusions/HeaderAnalysis.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include +#include namespace clang { namespace clangd { @@ -196,12 +205,14 @@ llvm::StringMap CachePathToFrameworkSpelling; llvm::StringMap CacheFrameworkToUmbrellaHeaderSpelling; + const include_cleaner::PragmaIncludes Π public: HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM, - const SymbolCollector::Options &Opts) - : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir) { - } + const SymbolCollector::Options &Opts, + const include_cleaner::PragmaIncludes &PI) + : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir), + PI(PI) {} // Returns a canonical URI for the file \p FE. // We attempt to make the path absolute first. @@ -377,6 +388,15 @@ if (!FE || FE->getName().empty()) return ""; llvm::StringRef Filename = FE->getName(); + + // Fallback pragma handling for Objective-C. + const auto *FileEntry = SM.getFileEntryForID(FID); + for (const auto *Export : PI.getExporters(FileEntry, SM.getFileManager())) + return toURI(Export->tryGetRealPathName()); + + if (auto Verbatim = PI.getPublic(FileEntry); !Verbatim.empty()) + return Verbatim; + // If a file is mapped by canonical headers, use that mapping, regardless // of whether it's an otherwise-good header (header guards etc). if (Includes) { @@ -436,7 +456,7 @@ void SymbolCollector::initialize(ASTContext &Ctx) { ASTCtx = &Ctx; HeaderFileURIs = std::make_unique( - this->PP, ASTCtx->getSourceManager(), Opts); + this->PP, ASTCtx->getSourceManager(), Opts, PI); CompletionAllocator = std::make_shared(); CompletionTUInfo = std::make_unique(CompletionAllocator); @@ -765,6 +785,11 @@ IndexedMacros.insert(Name); setIncludeLocation(S, DefLoc); + llvm::SmallVector Headers = + include_cleaner::headersForSymbol( + include_cleaner::Macro{const_cast(Name), DefLoc}, + ASTCtx->getSourceManager(), &PI); + setSymbolProviders(S, Headers); Symbols.insert(S); return true; } @@ -807,6 +832,14 @@ PP->getSourceManager().getDecomposedExpansionLoc(Loc).first; } +void SymbolCollector::setSymbolProviders( + const Symbol &S, + const llvm::SmallVector &Headers) { + if (Opts.CollectIncludePath && + shouldCollectIncludePath(S.SymInfo.Kind) != Symbol::Invalid) + SymbolProviders[S.ID] = Headers; +} + void SymbolCollector::finish() { // At the end of the TU, add 1 to the refcount of all referenced symbols. for (const auto &ID : ReferencedSymbols) { @@ -847,37 +880,77 @@ IncludeHeader = ""; } } - // Otherwise find the approprate include header for the defining file. - if (IncludeHeader.empty()) - IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); - - // Symbols in slabs aren't mutable, insert() has to walk all the strings - if (!IncludeHeader.empty()) { - Symbol::IncludeDirective Directives = Symbol::Invalid; - auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind); - if ((CollectDirectives & Symbol::Include) != 0) - Directives |= Symbol::Include; - // Only allow #import for symbols from ObjC-like files. - if ((CollectDirectives & Symbol::Import) != 0) { - auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID); - if (Inserted) - It->second = FilesWithObjCConstructs.contains(FID) || - tooling::codeContainsImports( - ASTCtx->getSourceManager().getBufferData(FID)); - if (It->second) - Directives |= Symbol::Import; - } - if (Directives != Symbol::Invalid) { - Symbol NewSym = *S; + + // Determine if the FID is #include'd or #import'ed. + Symbol::IncludeDirective Directives = Symbol::Invalid; + auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind); + if ((CollectDirectives & Symbol::Include) != 0) + Directives |= Symbol::Include; + // Only allow #import for symbols from ObjC-like files. + if ((CollectDirectives & Symbol::Import) != 0) { + auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID); + if (Inserted) + It->second = FilesWithObjCConstructs.contains(FID) || + tooling::codeContainsImports( + ASTCtx->getSourceManager().getBufferData(FID)); + if (It->second) + Directives |= Symbol::Import; + } + + Symbol NewSym; + if (Directives != Symbol::Invalid) { + NewSym = *S; + if (!IncludeHeader.empty()) { NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives}); Symbols.insert(NewSym); + continue; + } + } + + // Use the include location-based logic for Objective-C symbols. + if (Directives & Symbol::Import) { + // If not overridden manually, find the approprate include header for + // the defining file. + NewSym.IncludeHeaders.push_back( + {HeaderFileURIs->getIncludeHeader(FID), 1, Directives}); + Symbols.insert(NewSym); + continue; + } + + // For #include's, use the providers computed by the include-cleaner + // library. + if (Directives == Symbol::Include) { + auto It = SymbolProviders.find(SID); + if (It != SymbolProviders.end()) { + for (const auto &H : It->second) { + switch (H.kind()) { + case include_cleaner::Header::Kind::Verbatim: + SymbolIncludeSpelling[SID] = H.verbatim().str(); + break; + case include_cleaner::Header::Kind::Standard: + SymbolIncludeSpelling[SID] = H.standard().name().str(); + break; + case include_cleaner::Header::Kind::Physical: + SymbolIncludeSpelling[SID] = HeaderFileURIs->getIncludeHeader( + ASTCtx->getSourceManager().getOrCreateFileID( + H.physical(), SrcMgr::CharacteristicKind::C_User)); + break; + } + if (!SymbolIncludeSpelling[SID].empty()) { + NewSym.IncludeHeaders.push_back( + {SymbolIncludeSpelling[SID], 1, Directives}); + break; + } + } } + Symbols.insert(NewSym); } } } ReferencedSymbols.clear(); IncludeFiles.clear(); + SymbolProviders.clear(); FilesWithObjCConstructs.clear(); } @@ -952,6 +1025,10 @@ Symbols.insert(S); setIncludeLocation(S, ND.getLocation()); + llvm::SmallVector Headers = + include_cleaner::headersForSymbol(include_cleaner::Symbol{ND}, + ASTCtx->getSourceManager(), &PI); + setSymbolProviders(S, Headers); if (S.SymInfo.Lang == index::SymbolLanguage::ObjC) FilesWithObjCConstructs.insert(FID); return Symbols.find(S.ID); diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -9,6 +9,7 @@ #include "Annotations.h" #include "TestFS.h" #include "TestTU.h" +#include "URI.h" #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" @@ -226,23 +227,18 @@ class SymbolIndexActionFactory : public tooling::FrontendActionFactory { public: - SymbolIndexActionFactory(SymbolCollector::Options COpts, - CommentHandler *PragmaHandler) - : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {} + SymbolIndexActionFactory(SymbolCollector::Options COpts) + : COpts(std::move(COpts)) {} std::unique_ptr create() override { class IndexAction : public ASTFrontendAction { public: IndexAction(std::shared_ptr DataConsumer, - const index::IndexingOptions &Opts, - CommentHandler *PragmaHandler) - : DataConsumer(std::move(DataConsumer)), Opts(Opts), - PragmaHandler(PragmaHandler) {} + const index::IndexingOptions &Opts) + : DataConsumer(std::move(DataConsumer)), Opts(Opts) {} std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { - if (PragmaHandler) - CI.getPreprocessor().addCommentHandler(PragmaHandler); return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr()); } @@ -256,20 +252,17 @@ private: std::shared_ptr DataConsumer; index::IndexingOptions Opts; - CommentHandler *PragmaHandler; }; index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = true; Collector = std::make_shared(COpts); - return std::make_unique(Collector, std::move(IndexOpts), - PragmaHandler); + return std::make_unique(Collector, std::move(IndexOpts)); } std::shared_ptr Collector; SymbolCollector::Options COpts; - CommentHandler *PragmaHandler; }; class SymbolCollectorTest : public ::testing::Test { @@ -289,8 +282,7 @@ llvm::IntrusiveRefCntPtr Files( new FileManager(FileSystemOptions(), InMemoryFileSystem)); - auto Factory = std::make_unique( - CollectorOpts, PragmaHandler.get()); + auto Factory = std::make_unique(CollectorOpts); std::vector Args = {"symbol_collector", "-fsyntax-only", "-xc++", "-include", TestHeaderName}; @@ -324,7 +316,6 @@ RefSlab Refs; RelationSlab Relations; SymbolCollector::Options CollectorOpts; - std::unique_ptr PragmaHandler; }; TEST_F(SymbolCollectorTest, CollectSymbols) { @@ -1573,9 +1564,6 @@ TEST_F(SymbolCollectorTest, IWYUPragma) { CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - PragmaHandler = collectIWYUHeaderMaps(&Includes); - CollectorOpts.Includes = &Includes; const std::string Header = R"( // IWYU pragma: private, include the/good/header.h class Foo {}; @@ -1588,9 +1576,6 @@ TEST_F(SymbolCollectorTest, IWYUPragmaWithDoubleQuotes) { CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - PragmaHandler = collectIWYUHeaderMaps(&Includes); - CollectorOpts.Includes = &Includes; const std::string Header = R"( // IWYU pragma: private, include "the/good/header.h" class Foo {}; @@ -1601,6 +1586,27 @@ includeHeader("\"the/good/header.h\"")))); } +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once + #include "exporter.h" + )cpp"; + auto ExporterFile = testPath("exporter.h"); + InMemoryFileSystem->addFile( + ExporterFile, 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(#pragma once + #include "private.h" // IWYU pragma: export + )cpp")); + auto PrivateFile = testPath("private.h"); + InMemoryFileSystem->addFile( + PrivateFile, 0, llvm::MemoryBuffer::getMemBuffer("class Foo {};")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(ExporterFile).toString()), + declURI(URI::create(PrivateFile).toString())))); +} + TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) { auto IncFile = testPath("test.inc"); auto IncURI = URI::create(IncFile).toString();