diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -23,7 +23,7 @@ #include "SourceCode.h" #include "TUScheduler.h" #include "XRefs.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "index/FileIndex.h" #include "index/Merge.h" #include "index/StdLib.h" @@ -64,13 +64,13 @@ const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks, bool CollectInactiveRegions, const ClangdServer::Options &Opts) - : FIndex(FIndex), ServerCallbacks(ServerCallbacks), - TFS(TFS), Stdlib{std::make_shared()}, Tasks(Tasks), + : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS), + Stdlib{std::make_shared()}, Tasks(Tasks), CollectInactiveRegions(CollectInactiveRegions), Opts(Opts) {} void onPreambleAST( PathRef Path, llvm::StringRef Version, CapturedASTCtx ASTCtx, - const std::shared_ptr CanonIncludes) override { + std::shared_ptr PI) override { if (!FIndex) return; @@ -82,11 +82,10 @@ // FIndex outlives the UpdateIndexCallbacks. auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()), - ASTCtx(std::move(ASTCtx)), - CanonIncludes(CanonIncludes)]() mutable { + ASTCtx(std::move(ASTCtx)), PI]() mutable { trace::Span Tracer("PreambleIndexing"); FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(), - ASTCtx.getPreprocessor(), *CanonIncludes); + ASTCtx.getPreprocessor(), PI); }; if (Opts.AsyncPreambleIndexing && Tasks) { diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -21,7 +21,6 @@ #include "clang-include-cleaner/IncludeSpeller.h" #include "clang-include-cleaner/Types.h" #include "index/SymbolCollector.h" -#include "support/Logger.h" #include "support/Markup.h" #include "support/Trace.h" #include "clang/AST/ASTContext.h" @@ -1190,7 +1189,7 @@ const SourceManager &SM = AST.getSourceManager(); llvm::SmallVector RankedProviders = - include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes()); + include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes().get()); if (RankedProviders.empty()) return; @@ -1254,7 +1253,7 @@ llvm::DenseSet UsedSymbols; include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), collectMacroReferences(AST), - AST.getPragmaIncludes(), SM, + AST.getPragmaIncludes().get(), SM, [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { if (Ref.RT != include_cleaner::RefType::Explicit || diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -48,6 +48,7 @@ #include "llvm/Support/Regex.h" #include #include +#include #include #include #include @@ -85,8 +86,9 @@ return false; } -bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, - const include_cleaner::PragmaIncludes *PI) { +bool mayConsiderUnused( + const Inclusion &Inc, ParsedAST &AST, + std::shared_ptr PI) { // FIXME(kirillbobyrev): We currently do not support the umbrella headers. // System headers are likely to be standard library headers. // Until we have good support for umbrella headers, don't warn about them. @@ -421,7 +423,7 @@ trace::Span Tracer("include_cleaner::walkUsed"); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, - AST.getPragmaIncludes(), SM, + AST.getPragmaIncludes().get(), SM, [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { bool Satisfied = false; diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -26,7 +26,6 @@ #include "Headers.h" #include "Preamble.h" #include "clang-include-cleaner/Record.h" -#include "index/CanonicalIncludes.h" #include "support/Path.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Lex/Preprocessor.h" @@ -97,7 +96,6 @@ /// bytes. Does not include the size of the preamble. std::size_t getUsedBytes() const; const IncludeStructure &getIncludeStructure() const; - const CanonicalIncludes &getCanonicalIncludes() const; /// Gets all macro references (definition, expansions) present in the main /// file, including those in the preamble region. @@ -109,7 +107,8 @@ const syntax::TokenBuffer &getTokens() const { return Tokens; } /// Returns the PramaIncludes from the preamble. /// Might be null if AST is built without a preamble. - const include_cleaner::PragmaIncludes *getPragmaIncludes() const; + std::shared_ptr + getPragmaIncludes() const; /// Returns the version of the ParseInputs this AST was built from. llvm::StringRef version() const { return Version; } @@ -132,8 +131,7 @@ std::unique_ptr Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector Marks, std::vector LocalTopLevelDecls, - std::optional> Diags, IncludeStructure Includes, - CanonicalIncludes CanonIncludes); + std::optional> Diags, IncludeStructure Includes); Path TUPath; std::string Version; @@ -164,7 +162,6 @@ // top-level decls from the preamble. std::vector LocalTopLevelDecls; IncludeStructure Includes; - CanonicalIncludes CanonIncludes; std::unique_ptr Resolver; }; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -27,7 +27,6 @@ #include "SourceCode.h" #include "TidyProvider.h" #include "clang-include-cleaner/Record.h" -#include "index/CanonicalIncludes.h" #include "index/Symbol.h" #include "support/Logger.h" #include "support/Path.h" @@ -662,16 +661,8 @@ PP.addPPCallbacks( collectPragmaMarksCallback(Clang->getSourceManager(), Marks)); - // Copy over the includes from the preamble, then combine with the - // non-preamble includes below. - CanonicalIncludes CanonIncludes; - if (Preamble) - CanonIncludes = *Preamble->CanonIncludes; - else - CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts()); - std::unique_ptr IWYUHandler = - collectIWYUHeaderMaps(&CanonIncludes); - PP.addCommentHandler(IWYUHandler.get()); + // FIXME: Attach a comment handler to take care of + // keep/export/no_include etc. IWYU pragmas. // Collect tokens of the main file. syntax::TokenCollector CollectTokens(PP); @@ -729,8 +720,7 @@ ParsedAST Result(Filename, Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), std::move(Marks), std::move(ParsedDecls), - std::move(Diags), std::move(Includes), - std::move(CanonIncludes)); + std::move(Diags), std::move(Includes)); if (Result.Diags) llvm::move(getIncludeCleanerDiags(Result, Inputs.Contents), std::back_inserter(*Result.Diags)); @@ -817,10 +807,6 @@ return Includes; } -const CanonicalIncludes &ParsedAST::getCanonicalIncludes() const { - return CanonIncludes; -} - ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version, std::shared_ptr Preamble, std::unique_ptr Clang, @@ -829,22 +815,23 @@ std::vector Marks, std::vector LocalTopLevelDecls, std::optional> Diags, - IncludeStructure Includes, CanonicalIncludes CanonIncludes) + IncludeStructure Includes) : TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), Macros(std::move(Macros)), Marks(std::move(Marks)), Diags(std::move(Diags)), LocalTopLevelDecls(std::move(LocalTopLevelDecls)), - Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) { + Includes(std::move(Includes)) { Resolver = std::make_unique(getASTContext()); assert(this->Clang); assert(this->Action); } -const include_cleaner::PragmaIncludes *ParsedAST::getPragmaIncludes() const { +std::shared_ptr +ParsedAST::getPragmaIncludes() const { if (!Preamble) return nullptr; - return &Preamble->Pragmas; + return Preamble->Pragmas; } std::optional ParsedAST::preambleVersion() const { diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -28,7 +28,6 @@ #include "FS.h" #include "Headers.h" #include "clang-include-cleaner/Record.h" -#include "index/CanonicalIncludes.h" #include "support/Path.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/CompilerInvocation.h" @@ -104,7 +103,7 @@ // information, and their compile action skips preamble range. IncludeStructure Includes; // Captures #include-mapping information in #included headers. - include_cleaner::PragmaIncludes Pragmas; + std::shared_ptr Pragmas; // Macros defined in the preamble section of the main file. // Users care about headers vs main-file, not preamble vs non-preamble. // These should be treated as main-file entities e.g. for code completion. @@ -114,7 +113,6 @@ // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. std::shared_ptr StatCache; - std::shared_ptr CanonIncludes; // Whether there was a (possibly-incomplete) include-guard on the main file. // We need to propagate this information "by hand" to subsequent parses. bool MainIsIncludeGuarded = false; @@ -122,7 +120,7 @@ using PreambleParsedCallback = std::function CanonIncludes)>; + std::shared_ptr)>; /// Timings and statistics from the premble build. Unlike PreambleData, these /// do not need to be stored for later, but can be useful for logging, metrics, diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -15,7 +15,6 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang-include-cleaner/Record.h" -#include "index/CanonicalIncludes.h" #include "support/Logger.h" #include "support/Path.h" #include "support/ThreadsafeFS.h" @@ -95,7 +94,6 @@ include_cleaner::PragmaIncludes takePragmaIncludes() { return std::move(Pragmas); } - CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } std::optional takeLife() { return std::move(CapturedCtx); } @@ -141,7 +139,6 @@ } void BeforeExecute(CompilerInstance &CI) override { - CanonIncludes.addSystemHeadersMapping(CI.getLangOpts()); LangOpts = &CI.getLangOpts(); SourceMgr = &CI.getSourceManager(); PP = &CI.getPreprocessor(); @@ -160,11 +157,6 @@ collectPragmaMarksCallback(*SourceMgr, Marks)); } - CommentHandler *getCommentHandler() override { - IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes); - return IWYUHandler.get(); - } - static bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) { const auto *FD = FT->getTemplatedDecl(); const auto NumParams = FD->getNumParams(); @@ -214,12 +206,10 @@ private: PathRef File; IncludeStructure Includes; - CanonicalIncludes CanonIncludes; include_cleaner::PragmaIncludes Pragmas; MainFileMacros Macros; std::vector Marks; bool IsMainFileIncludeGuarded = false; - std::unique_ptr IWYUHandler = nullptr; const clang::LangOptions *LangOpts = nullptr; const SourceManager *SourceMgr = nullptr; const Preprocessor *PP = nullptr; @@ -686,11 +676,10 @@ Result->CompileCommand = Inputs.CompileCommand; Result->Diags = std::move(Diags); Result->Includes = CapturedInfo.takeIncludes(); - Result->Pragmas = CapturedInfo.takePragmaIncludes(); + Result->Pragmas = std::make_shared( + CapturedInfo.takePragmaIncludes()); Result->Macros = CapturedInfo.takeMacros(); Result->Marks = CapturedInfo.takeMarks(); - Result->CanonIncludes = std::make_shared( - (CapturedInfo.takeCanonicalIncludes())); Result->StatCache = StatCache; Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); if (PreambleCallback) { @@ -703,7 +692,7 @@ // While extending the life of FileMgr and VFS, StatCache should also be // extended. Ctx->setStatCache(Result->StatCache); - PreambleCallback(std::move(*Ctx), Result->CanonIncludes); + PreambleCallback(std::move(*Ctx), Result->Pragmas); } return Result; } diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -13,7 +13,7 @@ #include "Compiler.h" #include "Diagnostics.h" #include "GlobalCompilationDatabase.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "support/Function.h" #include "support/MemoryTree.h" #include "support/Path.h" @@ -21,6 +21,7 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include +#include #include #include @@ -161,10 +162,9 @@ /// Called on the AST that was built for emitting the preamble. The built AST /// contains only AST nodes from the #include directives at the start of the /// file. AST node in the current file should be observed on onMainAST call. - virtual void onPreambleAST(PathRef Path, llvm::StringRef Version, - CapturedASTCtx Ctx, - const std::shared_ptr) {} - + virtual void + onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx Ctx, + std::shared_ptr) {} /// The argument function is run under the critical section guarding against /// races when closing the files. using PublishFn = llvm::function_ref)>; diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -54,7 +54,7 @@ #include "GlobalCompilationDatabase.h" #include "ParsedAST.h" #include "Preamble.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "support/Cancellation.h" #include "support/Context.h" #include "support/Logger.h" @@ -1081,9 +1081,9 @@ LatestBuild = clang::clangd::buildPreamble( FileName, *Req.CI, Inputs, StoreInMemory, [&](CapturedASTCtx ASTCtx, - std::shared_ptr CanonIncludes) { + std::shared_ptr PI) { Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx), - CanonIncludes); + PI); }, &Stats); if (!LatestBuild) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1334,7 +1334,7 @@ auto ReferencedInclude = convertIncludes(SM, Inc); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), collectMacroReferences(AST), - AST.getPragmaIncludes(), SM, + AST.getPragmaIncludes().get(), SM, [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { if (Ref.RT != include_cleaner::RefType::Explicit) diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.h b/clang-tools-extra/clangd/index/CanonicalIncludes.h --- a/clang-tools-extra/clangd/index/CanonicalIncludes.h +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.h @@ -20,13 +20,9 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_CANONICALINCLUDES_H #include "clang/Basic/FileEntry.h" -#include "clang/Lex/Preprocessor.h" +#include "clang/Basic/LangOptions.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/FileSystem/UniqueID.h" -#include -#include -#include namespace clang { namespace clangd { @@ -36,17 +32,9 @@ /// Only const methods (i.e. mapHeader) in this class are thread safe. class CanonicalIncludes { public: - /// Adds a file-to-string mapping from \p ID to \p CanonicalPath. - void addMapping(FileEntryRef Header, llvm::StringRef CanonicalPath); - - /// Returns the overridden include for a qualified symbol with, or "". - /// \p Scope and \p Name concatenation forms the fully qualified name. - /// \p Scope is the qualifier with the trailing "::" (e.g. "std::") or empty - /// (for global namespace). - llvm::StringRef mapSymbol(llvm::StringRef Scope, llvm::StringRef Name, - const LangOptions &L) const; - - /// Returns the overridden include for files in \p Header, or "". + /// Returns the overridden verbatim spelling for files in \p Header that can + /// be directly included (i.e., contains quotes "" or angled brackets <>), or + /// "" if the spelling could not be found. llvm::StringRef mapHeader(FileEntryRef Header) const; /// Adds mapping for system headers and some special symbols (e.g. STL symbols @@ -60,37 +48,10 @@ void addSystemHeadersMapping(const LangOptions &Language); private: - /// A map from the private header to a canonical include path. - llvm::DenseMap FullPathMapping; /// A map from a suffix (one or components of a path) to a canonical path. /// Used only for mapping standard headers. const llvm::StringMap *StdSuffixHeaderMapping = nullptr; }; - -/// Returns a CommentHandler that parses pragma comment on include files to -/// determine when we should include a different header from the header that -/// directly defines a symbol. Mappinps are registered with \p Includes. -/// -/// Currently it only supports IWYU private pragma: -/// https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private -/// -/// We ignore other pragmas: -/// - keep: this is common but irrelevant: we do not currently remove includes -/// - export: this is common and potentially interesting, there are three cases: -/// * Points to a public header (common): we can suppress include2 if you -/// already have include1. Only marginally useful. -/// * Points to a private header annotated with `private` (somewhat common): -/// Not incrementally useful as we support private. -/// * Points to a private header without pragmas (rare). This is a reversed -/// private pragma, and is valuable but too rare to be worthwhile. -/// - no_include: this is about as common as private, but only affects the -/// current file, so the value is smaller. We could add support. -/// - friend: this is less common than private, has implementation difficulties, -/// and affects behavior in a limited scope. -/// - associated: extremely rare -std::unique_ptr -collectIWYUHeaderMaps(CanonicalIncludes *Includes); - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp --- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp @@ -7,14 +7,10 @@ //===----------------------------------------------------------------------===// #include "CanonicalIncludes.h" -#include "Headers.h" #include "clang/Basic/FileEntry.h" -#include "clang/Tooling/Inclusions/HeaderAnalysis.h" -#include "clang/Tooling/Inclusions/StandardLibrary.h" +#include "clang/Basic/LangOptions.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/FileSystem/UniqueID.h" #include "llvm/Support/Path.h" -#include namespace clang { namespace clangd { @@ -669,20 +665,11 @@ } // namespace -void CanonicalIncludes::addMapping(FileEntryRef Header, - llvm::StringRef CanonicalPath) { - FullPathMapping[Header.getUniqueID()] = std::string(CanonicalPath); -} - /// The maximum number of path components in a key from StdSuffixHeaderMapping. /// Used to minimize the number of lookups in suffix path mappings. constexpr int MaxSuffixComponents = 3; llvm::StringRef CanonicalIncludes::mapHeader(FileEntryRef Header) const { - auto MapIt = FullPathMapping.find(Header.getUniqueID()); - if (MapIt != FullPathMapping.end()) - return MapIt->second; - if (!StdSuffixHeaderMapping) return ""; @@ -701,54 +688,6 @@ return ""; } -llvm::StringRef CanonicalIncludes::mapSymbol(llvm::StringRef Scope, - llvm::StringRef Name, - const LangOptions &L) const { - tooling::stdlib::Lang Lang; - if (L.CPlusPlus) - Lang = tooling::stdlib::Lang::CXX; - else if (L.C11) - Lang = tooling::stdlib::Lang::C; - else - return ""; - // FIXME: remove the following special cases when the tooling stdlib supports - // them. - // There are two std::move()s, this is by far the most common. - if (Scope == "std::" && Name == "move") - return ""; - if (auto StdSym = tooling::stdlib::Symbol::named(Scope, Name, Lang)) - if (auto Header = StdSym->header()) - return Header->name(); - return ""; -} - -std::unique_ptr -collectIWYUHeaderMaps(CanonicalIncludes *Includes) { - class PragmaCommentHandler : public clang::CommentHandler { - public: - PragmaCommentHandler(CanonicalIncludes *Includes) : Includes(Includes) {} - - bool HandleComment(Preprocessor &PP, SourceRange Range) override { - auto Pragma = tooling::parseIWYUPragma( - PP.getSourceManager().getCharacterData(Range.getBegin())); - if (!Pragma || !Pragma->consume_front("private, include ")) - return false; - auto &SM = PP.getSourceManager(); - // We always insert using the spelling from the pragma. - if (auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))) - Includes->addMapping(FE->getLastRef(), - isLiteralInclude(*Pragma) - ? Pragma->str() - : ("\"" + *Pragma + "\"").str()); - return false; - } - - private: - CanonicalIncludes *const Includes; - }; - return std::make_unique(Includes); -} - void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) { // FIXME: remove the std header mapping once we support ambiguous symbols, now // it serves as a fallback to disambiguate: diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h --- a/clang-tools-extra/clangd/index/FileIndex.h +++ b/clang-tools-extra/clangd/index/FileIndex.h @@ -16,7 +16,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H #include "Headers.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "index/Index.h" #include "index/Merge.h" #include "index/Ref.h" @@ -112,8 +112,10 @@ /// Update preamble symbols of file \p Path with all declarations in \p AST /// and macros in \p PP. - void updatePreamble(PathRef Path, llvm::StringRef Version, ASTContext &AST, - Preprocessor &PP, const CanonicalIncludes &Includes); + void + updatePreamble(PathRef Path, llvm::StringRef Version, ASTContext &AST, + Preprocessor &PP, + std::shared_ptr PI); void updatePreamble(IndexFileIn); /// Update symbols and references from main file \p Path with @@ -160,9 +162,9 @@ /// Index declarations from \p AST and macros from \p PP that are declared in /// included headers. -SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, - Preprocessor &PP, - const CanonicalIncludes &Includes); +SlabTuple +indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, Preprocessor &PP, + std::shared_ptr PI); /// Takes slabs coming from a TU (multiple files) and shards them per /// declaration location. diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -9,7 +9,7 @@ #include "FileIndex.h" #include "CollectMacros.h" #include "ParsedAST.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "index/Index.h" #include "index/MemIndex.h" #include "index/Merge.h" @@ -43,14 +43,14 @@ namespace clangd { namespace { -SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP, - llvm::ArrayRef DeclsToIndex, - const MainFileMacros *MacroRefsToIndex, - const CanonicalIncludes &Includes, bool IsIndexMainAST, - llvm::StringRef Version, bool CollectMainFileRefs) { +SlabTuple indexSymbols( + ASTContext &AST, Preprocessor &PP, llvm::ArrayRef DeclsToIndex, + const MainFileMacros *MacroRefsToIndex, + std::shared_ptr PI, + bool IsIndexMainAST, llvm::StringRef Version, bool CollectMainFileRefs) { SymbolCollector::Options CollectorOpts; CollectorOpts.CollectIncludePath = true; - CollectorOpts.Includes = &Includes; + CollectorOpts.PragmaIncludes = PI; CollectorOpts.CountReferences = false; CollectorOpts.Origin = IsIndexMainAST ? SymbolOrigin::Open : SymbolOrigin::Preamble; @@ -222,18 +222,18 @@ SlabTuple indexMainDecls(ParsedAST &AST) { return indexSymbols( AST.getASTContext(), AST.getPreprocessor(), AST.getLocalTopLevelDecls(), - &AST.getMacros(), AST.getCanonicalIncludes(), + &AST.getMacros(), AST.getPragmaIncludes(), /*IsIndexMainAST=*/true, AST.version(), /*CollectMainFileRefs=*/true); } -SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, - Preprocessor &PP, - const CanonicalIncludes &Includes) { +SlabTuple +indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST, Preprocessor &PP, + std::shared_ptr PI) { std::vector DeclsToIndex( AST.getTranslationUnitDecl()->decls().begin(), AST.getTranslationUnitDecl()->decls().end()); return indexSymbols(AST, PP, DeclsToIndex, - /*MainFileMacros=*/nullptr, Includes, + /*MainFileMacros=*/nullptr, PI, /*IsIndexMainAST=*/false, Version, /*CollectMainFileRefs=*/false); } @@ -456,12 +456,12 @@ } } -void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version, - ASTContext &AST, Preprocessor &PP, - const CanonicalIncludes &Includes) { +void FileIndex::updatePreamble( + PathRef Path, llvm::StringRef Version, ASTContext &AST, Preprocessor &PP, + std::shared_ptr PI) { IndexFileIn IF; std::tie(IF.Symbols, std::ignore, IF.Relations) = - indexHeaderSymbols(Version, AST, PP, Includes); + indexHeaderSymbols(Version, AST, PP, PI); updatePreamble(std::move(IF)); } 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 @@ -9,7 +9,9 @@ #include "IndexAction.h" #include "AST.h" #include "Headers.h" +#include "clang-include-cleaner/Record.h" #include "index/Relation.h" +#include "index/SymbolCollector.h" #include "index/SymbolOrigin.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" @@ -126,7 +128,7 @@ class IndexAction : public ASTFrontendAction { public: IndexAction(std::shared_ptr C, - std::unique_ptr Includes, + std::shared_ptr PI, const index::IndexingOptions &Opts, std::function SymbolsCallback, std::function RefsCallback, @@ -134,9 +136,8 @@ std::function IncludeGraphCallback) : SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback), RelationsCallback(RelationsCallback), - IncludeGraphCallback(IncludeGraphCallback), Collector(C), - Includes(std::move(Includes)), Opts(Opts), - PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) { + IncludeGraphCallback(IncludeGraphCallback), Collector(C), PI(PI), + 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,8 +155,7 @@ std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { - CI.getPreprocessor().addCommentHandler(PragmaHandler.get()); - Includes->addSystemHeadersMapping(CI.getLangOpts()); + PI->record(CI.getPreprocessor()); if (IncludeGraphCallback != nullptr) CI.getPreprocessor().addPPCallbacks( std::make_unique(CI.getSourceManager(), IG)); @@ -201,9 +201,8 @@ std::function RelationsCallback; std::function IncludeGraphCallback; std::shared_ptr Collector; - std::unique_ptr Includes; + std::shared_ptr PI; index::IndexingOptions Opts; - std::unique_ptr PragmaHandler; IncludeGraph IG; }; @@ -228,12 +227,12 @@ Opts.RefFilter = RefKind::All; Opts.RefsInHeaders = true; } - auto Includes = std::make_unique(); - Opts.Includes = Includes.get(); - return std::make_unique( - std::make_shared(std::move(Opts)), std::move(Includes), - IndexOpts, SymbolsCallback, RefsCallback, RelationsCallback, - IncludeGraphCallback); + auto PragmaIncludes = std::make_shared(); + Opts.PragmaIncludes = PragmaIncludes; + return std::make_unique(std::make_shared(Opts), + std::move(PragmaIncludes), IndexOpts, + SymbolsCallback, RefsCallback, + RelationsCallback, IncludeGraphCallback); } } // namespace clangd 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" @@ -22,9 +24,10 @@ #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexSymbol.h" #include "clang/Sema/CodeCompleteConsumer.h" -#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" #include +#include #include namespace clang { @@ -57,8 +60,9 @@ std::string FallbackDir; bool CollectIncludePath = false; /// If set, this is used to map symbol #include path to a potentially - /// different #include path. - const CanonicalIncludes *Includes = nullptr; + /// different #include path specified by IWYU pragmas. + std::shared_ptr PragmaIncludes = + nullptr; // Populate the Symbol.References field. bool CountReferences = false; /// The symbol ref kinds that will be collected. @@ -123,8 +127,12 @@ void setPreprocessor(std::shared_ptr PP) override { this->PP = PP.get(); + SysHeaderMapping.addSystemHeadersMapping(PP->getLangOpts()); + } + void setPreprocessor(Preprocessor &PP) { + this->PP = &PP; + SysHeaderMapping.addSystemHeadersMapping(PP.getLangOpts()); } - void setPreprocessor(Preprocessor &PP) { this->PP = &PP; } bool handleDeclOccurrence(const Decl *D, index::SymbolRoleSet Roles, @@ -166,13 +174,21 @@ // All Symbols collected from the AST. SymbolSlab::Builder Symbols; - // File IDs for Symbol.IncludeHeaders. - // The final spelling is calculated in finish(). + // File IDs used to determine if the code contains Obj-C constructs. + // For Obj-C symbols, these File IDs are used to compute the include + // headers. llvm::DenseMap IncludeFiles; + void setIncludeLocation(const Symbol &S, SourceLocation, + const include_cleaner::Symbol &Sym); + + // Providers for Symbol.IncludeHeaders. + // The final spelling is calculated in finish(). + llvm::DenseMap SymbolProviders; + // Files which contain ObjC symbols. // This is finalized and used in finish(). llvm::DenseSet FilesWithObjCConstructs; - void setIncludeLocation(const Symbol &S, SourceLocation); + // 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 +200,8 @@ RelationSlab::Builder Relations; ASTContext *ASTCtx; Preprocessor *PP = nullptr; + // This is used to map symbols to their #include paths in system headers. + CanonicalIncludes SysHeaderMapping; 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" @@ -22,6 +26,7 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclarationName.h" +#include "clang/Basic/FileEntry.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -30,10 +35,17 @@ #include "clang/Lex/Token.h" #include "clang/Tooling/Inclusions/HeaderAnalysis.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/Support/Casting.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" +#include +#include #include +#include +#include namespace clang { namespace clangd { @@ -188,7 +200,8 @@ // (IndexDataConsumer::setPreprocessor can happen before or after initialize) Preprocessor *&PP; const SourceManager &SM; - const CanonicalIncludes *Includes; + const CanonicalIncludes *SysHeaderMapping; + std::shared_ptr PI; llvm::StringRef FallbackDir; llvm::DenseMap CacheFEToURI; llvm::StringMap CachePathToURI; @@ -199,9 +212,10 @@ public: HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM, + const CanonicalIncludes *SysHeaderMapping, const SymbolCollector::Options &Opts) - : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir) { - } + : PP(PP), SM(SM), SysHeaderMapping(SysHeaderMapping), + PI(Opts.PragmaIncludes), FallbackDir(Opts.FallbackDir) {} // Returns a canonical URI for the file \p FE. // We attempt to make the path absolute first. @@ -234,6 +248,19 @@ return R.first->second; } + // If a file is mapped by canonical headers, use that mapping, regardless + // of whether it's an otherwise-good header (header guards etc). + llvm::StringRef mapCanonical(FileEntryRef FE) { + if (!SysHeaderMapping) + return ""; + auto Canonical = SysHeaderMapping->mapHeader(FE); + if (Canonical.empty()) + return ""; + // If we had a mapping, always use it. + assert(Canonical.startswith("<") || Canonical.startswith("\"")); + return Canonical; + } + private: // This takes care of making paths absolute and path->URI caching, but no // FileManager-based canonicalization. @@ -376,19 +403,14 @@ const auto FE = SM.getFileEntryRefForID(FID); if (!FE || FE->getName().empty()) return ""; - llvm::StringRef Filename = FE->getName(); - // 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) { - llvm::StringRef Canonical = - Includes->mapHeader(*SM.getFileEntryRefForID(FID)); - if (!Canonical.empty()) { - // If we had a mapping, always use it. - if (Canonical.startswith("<") || Canonical.startswith("\"")) - return Canonical; - return toURI(Canonical); - } - } + + if (auto Verbatim = PI->getPublic(SM.getFileEntryForID(FID)); + !Verbatim.empty()) + return Verbatim; + + if (auto Canonical = mapCanonical(*FE); !Canonical.empty()) + return Canonical; + // Framework headers are spelled as , not // "path/FrameworkName.framework/Headers/Foo.h". auto &HS = PP->getHeaderSearchInfo(); @@ -398,6 +420,7 @@ getFrameworkHeaderIncludeSpelling(*FE, HFI->Framework, HS)) return *Spelling; + llvm::StringRef Filename = FE->getName(); if (!tooling::isSelfContainedHeader(*FE, PP->getSourceManager(), PP->getHeaderSearchInfo())) { // A .inc or .def file is often included into a real header to define @@ -436,7 +459,7 @@ void SymbolCollector::initialize(ASTContext &Ctx) { ASTCtx = &Ctx; HeaderFileURIs = std::make_unique( - this->PP, ASTCtx->getSourceManager(), Opts); + this->PP, ASTCtx->getSourceManager(), &SysHeaderMapping, Opts); CompletionAllocator = std::make_shared(); CompletionTUInfo = std::make_unique(CompletionAllocator); @@ -764,7 +787,10 @@ S.CompletionSnippetSuffix = SnippetSuffix; IndexedMacros.insert(Name); - setIncludeLocation(S, DefLoc); + + setIncludeLocation( + S, DefLoc, + include_cleaner::Macro{const_cast(Name), DefLoc}); Symbols.insert(S); return true; } @@ -798,13 +824,19 @@ } } -void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation Loc) { +void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation DefLoc, + const include_cleaner::Symbol &Sym) { + const auto &SM = PP->getSourceManager(); if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind) != Symbol::Invalid) // Use the expansion location to get the #include header since this is // where the symbol is exposed. - IncludeFiles[S.ID] = - PP->getSourceManager().getDecomposedExpansionLoc(Loc).first; + IncludeFiles[S.ID] = SM.getDecomposedExpansionLoc(DefLoc).first; + + llvm::SmallVector Headers = + include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes.get()); + if (!Headers.empty()) + SymbolProviders.insert({S.ID, Headers[0]}); } void SymbolCollector::finish() { @@ -833,51 +865,81 @@ // Fill in IncludeHeaders. // We delay this until end of TU so header guards are all resolved. for (const auto &[SID, FID] : IncludeFiles) { - if (const Symbol *S = Symbols.find(SID)) { - llvm::StringRef IncludeHeader; - // Look for an overridden include header for this symbol specifically. - if (Opts.Includes) { - IncludeHeader = - Opts.Includes->mapSymbol(S->Scope, S->Name, ASTCtx->getLangOpts()); - if (!IncludeHeader.empty()) { - if (IncludeHeader.front() != '"' && IncludeHeader.front() != '<') - IncludeHeader = HeaderFileURIs->toURI(IncludeHeader); - else if (IncludeHeader == "" && S->Scope == "std::" && - S->Name == "move" && S->Signature.contains(',')) - 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; - NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives}); - Symbols.insert(NewSym); - } + const Symbol *S = Symbols.find(SID); + if (!S) + continue; + // 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; + } + + if (Directives == Symbol::Invalid) + continue; + + auto NewSym = *S; + // Use the include location-based logic for Objective-C symbols. + if (Directives & Symbol::Import) { + if (auto IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); + !IncludeHeader.empty()) { + NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives}); + Symbols.insert(NewSym); } + // FIXME: use providers from include-cleaner library once it's polished + // for Objective-C. + continue; + } + + assert(Directives == Symbol::Include); + // For #include's, use the providers computed by the include-cleaner + // library. + auto It = SymbolProviders.find(SID); + if (It == SymbolProviders.end()) + continue; + const auto &H = It->second; + llvm::DenseMap HeaderSpelling; + const auto [SpellingIt, Inserted] = HeaderSpelling.try_emplace(H); + if (!Inserted) + continue; + switch (H.kind()) { + case include_cleaner::Header::Kind::Verbatim: + SpellingIt->second = H.verbatim().str(); + break; + case include_cleaner::Header::Kind::Standard: + SpellingIt->second = H.standard().name().str(); + break; + case include_cleaner::Header::Kind::Physical: + auto &SM = ASTCtx->getSourceManager(); + if (auto Canonical = + HeaderFileURIs->mapCanonical(H.physical()->getLastRef()); + !Canonical.empty()) + SpellingIt->second = Canonical; + else if (!tooling::isSelfContainedHeader(H.physical(), SM, + PP->getHeaderSearchInfo())) + break; + else + SpellingIt->second = + HeaderFileURIs->toURI(H.physical()->tryGetRealPathName()); + break; } + if (!SpellingIt->second.empty()) + NewSym.IncludeHeaders.push_back({SpellingIt->second, 1, Directives}); + Symbols.insert(NewSym); } ReferencedSymbols.clear(); IncludeFiles.clear(); + SymbolProviders.clear(); FilesWithObjCConstructs.clear(); } @@ -951,7 +1013,7 @@ } Symbols.insert(S); - setIncludeLocation(S, ND.getLocation()); + setIncludeLocation(S, ND.getLocation(), include_cleaner::Symbol{ND}); if (S.SymInfo.Lang == index::SymbolLanguage::ObjC) FilesWithObjCConstructs.insert(FID); return Symbols.find(S.ID); diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -47,7 +47,7 @@ #include "SourceCode.h" #include "TidyProvider.h" #include "XRefs.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "index/FileIndex.h" #include "refactor/Tweak.h" #include "support/Context.h" @@ -231,12 +231,12 @@ Preamble = buildPreamble( File, *Invocation, Inputs, /*StoreInMemory=*/true, [&](CapturedASTCtx Ctx, - const std::shared_ptr Includes) { + std::shared_ptr PI) { if (!Opts.BuildDynamicSymbolIndex) return; log("Indexing headers..."); Index.updatePreamble(File, /*Version=*/"null", Ctx.getASTContext(), - Ctx.getPreprocessor(), *Includes); + Ctx.getPreprocessor(), PI); }); if (!Preamble) { elog("Failed to build preamble"); diff --git a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp --- a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp +++ b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp @@ -6,10 +6,10 @@ // //===----------------------------------------------------------------------===// -#include "TestFS.h" #include "index/CanonicalIncludes.h" #include "clang/Basic/FileEntry.h" #include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemOptions.h" #include "clang/Basic/LangOptions.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/StringRef.h" @@ -30,76 +30,15 @@ return *File; } -TEST(CanonicalIncludesTest, CStandardLibrary) { - CanonicalIncludes CI; - auto Language = LangOptions(); - Language.C11 = true; - CI.addSystemHeadersMapping(Language); - // Usual standard library symbols are mapped correctly. - EXPECT_EQ("", CI.mapSymbol("", "printf", Language)); - EXPECT_EQ("", CI.mapSymbol("", "unknown_symbol", Language)); -} - -TEST(CanonicalIncludesTest, CXXStandardLibrary) { - CanonicalIncludes CI; - auto Language = LangOptions(); - Language.CPlusPlus = true; - CI.addSystemHeadersMapping(Language); - - // Usual standard library symbols are mapped correctly. - EXPECT_EQ("", CI.mapSymbol("std::", "vector", Language)); - EXPECT_EQ("", CI.mapSymbol("std::", "printf", Language)); - // std::move is ambiguous, currently always mapped to - EXPECT_EQ("", CI.mapSymbol("std::", "move", Language)); - EXPECT_EQ("", CI.mapSymbol("std::", "size_t", Language)); - // Unknown std symbols aren't mapped. - EXPECT_EQ("", CI.mapSymbol("std::", "notathing", Language)); - // iosfwd declares some symbols it doesn't own. - EXPECT_EQ("", CI.mapSymbol("std::", "ostream", Language)); - // And (for now) we assume it owns the others. - auto InMemFS = llvm::makeIntrusiveRefCnt(); - FileManager Files(FileSystemOptions(), InMemFS); - auto File = addFile(*InMemFS, Files, testPath("iosfwd")); - EXPECT_EQ("", CI.mapHeader(File)); -} - -TEST(CanonicalIncludesTest, PathMapping) { - auto InMemFS = llvm::makeIntrusiveRefCnt(); - FileManager Files(FileSystemOptions(), InMemFS); - std::string BarPath = testPath("foo/bar"); - auto Bar = addFile(*InMemFS, Files, BarPath); - auto Other = addFile(*InMemFS, Files, testPath("foo/baz")); - // As used for IWYU pragmas. - CanonicalIncludes CI; - CI.addMapping(Bar, ""); - - // We added a mapping for baz. - EXPECT_EQ("", CI.mapHeader(Bar)); - // Other file doesn't have a mapping. - EXPECT_EQ("", CI.mapHeader(Other)); - - // Add hard link to "foo/bar" and check that it is also mapped to , hence - // does not depend on the header name. - std::string HardLinkPath = testPath("hard/link"); - InMemFS->addHardLink(HardLinkPath, BarPath); - auto HardLinkFile = Files.getFileRef(HardLinkPath); - ASSERT_THAT_EXPECTED(HardLinkFile, llvm::Succeeded()); - EXPECT_EQ("", CI.mapHeader(*HardLinkFile)); -} - -TEST(CanonicalIncludesTest, Precedence) { +TEST(CanonicalIncludesTest, SystemHeaderMap) { auto InMemFS = llvm::makeIntrusiveRefCnt(); FileManager Files(FileSystemOptions(), InMemFS); - auto File = addFile(*InMemFS, Files, testPath("some/path")); CanonicalIncludes CI; - CI.addMapping(File, ""); LangOptions Language; Language.CPlusPlus = true; CI.addSystemHeadersMapping(Language); - // We added a mapping from some/path to . - ASSERT_EQ("", CI.mapHeader(File)); // We should have a path from 'bits/stl_vector.h' to ''. // FIXME: The Standrad Library map in CanonicalIncludes expects forward // slashes and Windows would use backward slashes instead, so the headers are diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -15,7 +15,7 @@ #include "TestTU.h" #include "TestWorkspace.h" #include "URI.h" -#include "index/CanonicalIncludes.h" +#include "clang-include-cleaner/Record.h" #include "index/FileIndex.h" #include "index/Index.h" #include "index/Ref.h" @@ -25,12 +25,12 @@ #include "index/SymbolID.h" #include "support/Threading.h" #include "clang/Frontend/CompilerInvocation.h" -#include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/Allocator.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include #include #include @@ -60,6 +60,11 @@ MATCHER_P(numReferences, N, "") { return arg.References == N; } MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); } +MATCHER_P(includeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} + namespace clang { namespace clangd { namespace { @@ -171,7 +176,7 @@ auto AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); } TEST(FileIndexTest, CustomizedURIScheme) { @@ -234,6 +239,32 @@ ""); } +TEST(FileIndexTest, IWYUPragmaExport) { + FileIndex M; + + TestTU File; + File.Code = R"cpp(#pragma once + #include "exporter.h" + )cpp"; + File.HeaderFilename = "exporter.h"; + File.HeaderCode = R"cpp(#pragma once + #include "private.h" // IWYU pragma: export + )cpp"; + File.AdditionalFiles["private.h"] = "class Foo{};"; + auto AST = File.build(); + M.updatePreamble(testPath(File.Filename), /*Version=*/"null", + AST.getASTContext(), AST.getPreprocessor(), + AST.getPragmaIncludes()); + + auto Symbols = runFuzzyFind(M, ""); + EXPECT_THAT( + Symbols, + UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(testPath(File.HeaderFilename)).toString()), + declURI(URI::create(testPath("private.h")).toString())))); +} + TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) { TestTU TU; TU.HeaderCode = "class Foo{};"; @@ -305,18 +336,17 @@ FileIndex Index; bool IndexUpdated = false; - buildPreamble( - FooCpp, *CI, PI, - /*StoreInMemory=*/true, - [&](CapturedASTCtx ASTCtx, - const std::shared_ptr CanonIncludes) { - auto &Ctx = ASTCtx.getASTContext(); - auto &PP = ASTCtx.getPreprocessor(); - EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; - IndexUpdated = true; - Index.updatePreamble(FooCpp, /*Version=*/"null", Ctx, PP, - *CanonIncludes); - }); + buildPreamble(FooCpp, *CI, PI, + /*StoreInMemory=*/true, + [&](CapturedASTCtx ASTCtx, + std::shared_ptr PI) { + auto &Ctx = ASTCtx.getASTContext(); + auto &PP = ASTCtx.getPreprocessor(); + EXPECT_FALSE(IndexUpdated) + << "Expected only a single index update"; + IndexUpdated = true; + Index.updatePreamble(FooCpp, /*Version=*/"null", Ctx, PP, PI); + }); ASSERT_TRUE(IndexUpdated); // Check the index contains symbols from the preamble, but not from the main @@ -416,7 +446,7 @@ FileIndex Index; Index.updatePreamble(testPath(TU.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); SymbolID A = findSymbol(TU.headerSymbols(), "A").ID; uint32_t Results = 0; RelationsRequest Req; @@ -537,7 +567,7 @@ auto AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); EXPECT_THAT(runFuzzyFind(M, ""), UnorderedElementsAre(qName("a"))); File.Filename = "f2.cpp"; @@ -545,7 +575,7 @@ AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); EXPECT_THAT(runFuzzyFind(M, ""), UnorderedElementsAre(qName("b"))); } @@ -690,7 +720,7 @@ auto AST = TestTU::withHeaderCode("int a;").build(); FI.updateMain(FileName, AST); FI.updatePreamble(FileName, "v1", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); llvm::BumpPtrAllocator Alloc; MemoryTree MT(&Alloc); 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,8 @@ #include "Annotations.h" #include "TestFS.h" #include "TestTU.h" +#include "URI.h" +#include "clang-include-cleaner/Record.h" #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" @@ -20,7 +22,6 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" -#include "llvm/Testing/Support/Error.h" #include "gmock/gmock-matchers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -28,6 +29,7 @@ #include #include #include +#include namespace clang { namespace clangd { @@ -226,23 +228,21 @@ 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) + std::shared_ptr PI) : DataConsumer(std::move(DataConsumer)), Opts(Opts), - PragmaHandler(PragmaHandler) {} + PI(std::move(PI)) {} std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { - if (PragmaHandler) - CI.getPreprocessor().addCommentHandler(PragmaHandler); + PI->record(CI); return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr()); } @@ -256,20 +256,22 @@ private: std::shared_ptr DataConsumer; index::IndexingOptions Opts; - CommentHandler *PragmaHandler; + std::shared_ptr PI; }; index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = true; + std::shared_ptr PI = + std::make_shared(); + COpts.PragmaIncludes = PI; Collector = std::make_shared(COpts); return std::make_unique(Collector, std::move(IndexOpts), - PragmaHandler); + std::move(PI)); } std::shared_ptr Collector; SymbolCollector::Options COpts; - CommentHandler *PragmaHandler; }; class SymbolCollectorTest : public ::testing::Test { @@ -289,8 +291,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 +325,6 @@ RefSlab Refs; RelationSlab Relations; SymbolCollector::Options CollectorOpts; - std::unique_ptr PragmaHandler; }; TEST_F(SymbolCollectorTest, CollectSymbols) { @@ -1545,11 +1545,6 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) { CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - auto Language = LangOptions(); - Language.CPlusPlus = true; - Includes.addSystemHeadersMapping(Language); - CollectorOpts.Includes = &Includes; runSymbolCollector( R"cpp( namespace std { @@ -1573,9 +1568,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 +1580,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,29 +1590,25 @@ includeHeader("\"the/good/header.h\"")))); } -TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) { - auto IncFile = testPath("test.inc"); - auto IncURI = URI::create(IncFile).toString(); - InMemoryFileSystem->addFile(IncFile, 0, - llvm::MemoryBuffer::getMemBuffer("class X {};")); - llvm::IntrusiveRefCntPtr Files( - new FileManager(FileSystemOptions(), InMemoryFileSystem)); - std::string HeaderCode = "#include \"test.inc\"\nclass Y {};"; - InMemoryFileSystem->addFile(TestHeaderName, 0, - llvm::MemoryBuffer::getMemBuffer(HeaderCode)); - auto File = Files->getFileRef(TestHeaderName); - ASSERT_THAT_EXPECTED(File, llvm::Succeeded()); - CanonicalIncludes Includes; - Includes.addMapping(*File, ""); +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; - CollectorOpts.Includes = &Includes; - runSymbolCollector(HeaderCode, /*Main=*/"", + 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("X"), declURI(IncURI), - includeHeader("")), - AllOf(qName("Y"), declURI(TestHeaderURI), - includeHeader("")))); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(ExporterFile).toString()), + declURI(URI::create(PrivateFile).toString())))); } TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) { diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -18,6 +18,7 @@ #include "TUScheduler.h" #include "TestFS.h" #include "TestIndex.h" +#include "clang-include-cleaner/Record.h" #include "support/Cancellation.h" #include "support/Context.h" #include "support/Path.h" @@ -1129,9 +1130,9 @@ public: BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N) : BlockVersion(BlockVersion), N(N) {} - void - onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, - const std::shared_ptr) override { + void onPreambleAST( + PathRef Path, llvm::StringRef Version, CapturedASTCtx, + std::shared_ptr) override { if (Version == BlockVersion) N.wait(); } @@ -1208,9 +1209,9 @@ BlockPreambleThread(Notification &UnblockPreamble, DiagsCB CB) : UnblockPreamble(UnblockPreamble), CB(std::move(CB)) {} - void - onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, - const std::shared_ptr) override { + void onPreambleAST( + PathRef Path, llvm::StringRef Version, CapturedASTCtx, + std::shared_ptr) override { if (BuildBefore) ASSERT_TRUE(UnblockPreamble.wait(timeoutSeconds(5))) << "Expected notification"; @@ -1562,9 +1563,9 @@ std::vector &Filenames; CaptureBuiltFilenames(std::vector &Filenames) : Filenames(Filenames) {} - void - onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, - const std::shared_ptr) override { + void onPreambleAST( + PathRef Path, llvm::StringRef Version, CapturedASTCtx, + std::shared_ptr PI) override { // Deliberately no synchronization. // The PreambleThrottler should serialize these calls, if not then tsan // will find a bug here. diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -164,9 +164,9 @@ SymbolSlab TestTU::headerSymbols() const { auto AST = build(); - return std::get<0>(indexHeaderSymbols(/*Version=*/"null", AST.getASTContext(), - AST.getPreprocessor(), - AST.getCanonicalIncludes())); + return std::get<0>(indexHeaderSymbols( + /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), + AST.getPragmaIncludes())); } RefSlab TestTU::headerRefs() const { @@ -179,7 +179,7 @@ auto Idx = std::make_unique(); Idx->updatePreamble(testPath(Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - AST.getCanonicalIncludes()); + AST.getPragmaIncludes()); Idx->updateMain(testPath(Filename), AST); return std::move(Idx); } diff --git a/clang-tools-extra/clangd/unittests/TestWorkspace.cpp b/clang-tools-extra/clangd/unittests/TestWorkspace.cpp --- a/clang-tools-extra/clangd/unittests/TestWorkspace.cpp +++ b/clang-tools-extra/clangd/unittests/TestWorkspace.cpp @@ -7,8 +7,10 @@ //===----------------------------------------------------------------------===// #include "TestWorkspace.h" +#include "clang-include-cleaner/Record.h" #include "index/FileIndex.h" #include "gtest/gtest.h" +#include #include namespace clang { @@ -21,14 +23,12 @@ continue; TU.Code = Input.second.Code; TU.Filename = Input.first().str(); - TU.preamble( - [&](CapturedASTCtx ASTCtx, - const std::shared_ptr CanonIncludes) { - auto &Ctx = ASTCtx.getASTContext(); - auto &PP = ASTCtx.getPreprocessor(); - Index->updatePreamble(testPath(Input.first()), "null", Ctx, PP, - *CanonIncludes); - }); + TU.preamble([&](CapturedASTCtx ASTCtx, + std::shared_ptr PI) { + auto &Ctx = ASTCtx.getASTContext(); + auto &PP = ASTCtx.getPreprocessor(); + Index->updatePreamble(testPath(Input.first()), "null", Ctx, PP, PI); + }); ParsedAST MainAST = TU.build(); Index->updateMain(testPath(Input.first()), MainAST); }