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 @@ -367,7 +367,7 @@ if (Preamble) CanonIncludes = Preamble->CanonIncludes; else - addSystemHeadersMapping(&CanonIncludes, Clang->getLangOpts()); + CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts()); std::unique_ptr IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes); Clang->getPreprocessor().addCommentHandler(IWYUHandler.get()); 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 @@ -78,7 +78,7 @@ } void BeforeExecute(CompilerInstance &CI) override { - addSystemHeadersMapping(&CanonIncludes, CI.getLangOpts()); + CanonIncludes.addSystemHeadersMapping(CI.getLangOpts()); SourceMgr = &CI.getSourceManager(); } 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 @@ -34,37 +34,37 @@ /// Only const methods (i.e. mapHeader) in this class are thread safe. class CanonicalIncludes { public: - CanonicalIncludes() = default; - /// Adds a string-to-string mapping from \p Path to \p CanonicalPath. void addMapping(llvm::StringRef Path, llvm::StringRef CanonicalPath); - /// Maps files with last path components matching \p Suffix to \p - /// CanonicalPath. - void addPathSuffixMapping(llvm::StringRef Suffix, - llvm::StringRef CanonicalPath); - - /// Sets the canonical include for any symbol with \p QualifiedName. - /// Symbol mappings take precedence over header mappings. - void addSymbolMapping(llvm::StringRef QualifiedName, - llvm::StringRef CanonicalPath); - /// Returns the canonical include for symbol with \p QualifiedName. /// \p Header is the file the declaration was reachable from. /// Header itself will be returned if there is no relevant mapping. llvm::StringRef mapHeader(llvm::StringRef Header, llvm::StringRef QualifiedName) const; + /// Adds mapping for system headers and some special symbols (e.g. STL symbols + /// in need to be mapped individually). Approximately, the following + /// system headers are handled: + /// - C++ standard library e.g. bits/basic_string.h$ -> + /// - Posix library e.g. bits/pthreadtypes.h$ -> + /// - Compiler extensions, e.g. include/avx512bwintrin.h$ -> + /// The mapping is hardcoded and hand-maintained, so it might not cover all + /// headers. + void addSystemHeadersMapping(const LangOptions &Language); + private: /// A map from full include path to a canonical path. llvm::StringMap FullPathMapping; /// A map from a suffix (one or components of a path) to a canonical path. - llvm::StringMap SuffixHeaderMapping; + /// Used only for mapping standard headers. + const llvm::StringMap *SuffixHeaderMapping = nullptr; /// Maximum number of path components stored in a key of SuffixHeaderMapping. /// Used to reduce the number of lookups into SuffixHeaderMapping. int MaxSuffixComponents = 0; /// A map from fully qualified symbol names to header names. - llvm::StringMap SymbolMapping; + /// Used only for mapping standard symbols. + const llvm::StringMap *SymbolMapping = nullptr; }; /// Returns a CommentHandler that parses pragma comment on include files to @@ -76,17 +76,6 @@ std::unique_ptr collectIWYUHeaderMaps(CanonicalIncludes *Includes); -/// Adds mapping for system headers and some special symbols (e.g. STL symbols -/// in need to be mapped individually). Approximately, the following -/// system headers are handled: -/// - C++ standard library e.g. bits/basic_string.h$ -> -/// - Posix library e.g. bits/pthreadtypes.h$ -> -/// - Compiler extensions, e.g. include/avx512bwintrin.h$ -> -/// The mapping is hardcoded and hand-maintained, so it might not cover all -/// headers. -void addSystemHeadersMapping(CanonicalIncludes *Includes, - const LangOptions &Language); - } // 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 @@ -9,6 +9,7 @@ #include "CanonicalIncludes.h" #include "Headers.h" #include "clang/Driver/Types.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Path.h" #include @@ -18,43 +19,35 @@ const char IWYUPragma[] = "// IWYU pragma: private, include "; } // namespace -void CanonicalIncludes::addPathSuffixMapping(llvm::StringRef Suffix, - llvm::StringRef CanonicalPath) { - int Components = std::distance(llvm::sys::path::begin(Suffix), - llvm::sys::path::end(Suffix)); - MaxSuffixComponents = std::max(MaxSuffixComponents, Components); - SuffixHeaderMapping[Suffix] = CanonicalPath; -} - void CanonicalIncludes::addMapping(llvm::StringRef Path, llvm::StringRef CanonicalPath) { FullPathMapping[Path] = CanonicalPath; } -void CanonicalIncludes::addSymbolMapping(llvm::StringRef QualifiedName, - llvm::StringRef CanonicalPath) { - this->SymbolMapping[QualifiedName] = CanonicalPath; -} - llvm::StringRef CanonicalIncludes::mapHeader(llvm::StringRef Header, llvm::StringRef QualifiedName) const { assert(!Header.empty()); - auto SE = SymbolMapping.find(QualifiedName); - if (SE != SymbolMapping.end()) - return SE->second; + if (SymbolMapping) { + auto SE = SymbolMapping->find(QualifiedName); + if (SE != SymbolMapping->end()) + return SE->second; + } auto MapIt = FullPathMapping.find(Header); if (MapIt != FullPathMapping.end()) return MapIt->second; + if (!SuffixHeaderMapping) + return Header; + int Components = 1; for (auto It = llvm::sys::path::rbegin(Header), End = llvm::sys::path::rend(Header); It != End && Components <= MaxSuffixComponents; ++It, ++Components) { auto SubPath = Header.substr(It->data() - Header.begin()); - auto MappingIt = SuffixHeaderMapping.find(SubPath); - if (MappingIt != SuffixHeaderMapping.end()) + auto MappingIt = SuffixHeaderMapping->find(SubPath); + if (MappingIt != SuffixHeaderMapping->end()) return MappingIt->second; } return Header; @@ -86,25 +79,12 @@ return std::make_unique(Includes); } -void addSystemHeadersMapping(CanonicalIncludes *Includes, - const LangOptions &Language) { - static constexpr std::pair SymbolMap[] = { -#define SYMBOL(Name, NameSpace, Header) { #NameSpace#Name, #Header }, - #include "StdSymbolMap.inc" -#undef SYMBOL - }; - static constexpr std::pair CSymbolMap[] = { -#define SYMBOL(Name, NameSpace, Header) { #Name, #Header }, - #include "CSymbolMap.inc" -#undef SYMBOL - }; - if (Language.CPlusPlus) { - for (const auto &Pair : SymbolMap) - Includes->addSymbolMapping(Pair.first, Pair.second); - } else if (Language.C11) { - for (const auto &Pair : CSymbolMap) - Includes->addSymbolMapping(Pair.first, Pair.second); - } +struct PathSuffixMap { + llvm::StringMap SuffixHeaderMapping; + int MaxSuffixComponents = 0; +}; + +PathSuffixMap buildSuffixMap() { // FIXME: remove the std header mapping once we support ambiguous symbols, now // it serves as a fallback to disambiguate: // - symbols with multiple headers (e.g. std::move) @@ -761,8 +741,51 @@ {"bits/waitstatus.h", ""}, {"bits/xtitypes.h", ""}, }; - for (const auto &Pair : SystemHeaderMap) - Includes->addPathSuffixMapping(Pair.first, Pair.second); + PathSuffixMap Includes; + for (const auto &Pair : SystemHeaderMap) { + llvm::StringRef Suffix = Pair.first; + llvm::StringRef CanonicalPath = Pair.second; + int Components = std::distance(llvm::sys::path::begin(Suffix), + llvm::sys::path::end(Suffix)); + Includes.MaxSuffixComponents = + std::max(Includes.MaxSuffixComponents, Components); + Includes.SuffixHeaderMapping[Suffix] = CanonicalPath; + } + return Includes; +} + +void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) { + static constexpr std::pair SymbolMap[] = { +#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name, #Header}, +#include "StdSymbolMap.inc" +#undef SYMBOL + }; + static constexpr std::pair CSymbolMap[] = { +#define SYMBOL(Name, NameSpace, Header) {#Name, #Header}, +#include "CSymbolMap.inc" +#undef SYMBOL + }; + if (Language.CPlusPlus) { + static const auto *Symbols = []() { + auto Symbols = std::make_unique>(); + for (const auto &Pair : SymbolMap) + (*Symbols)[Pair.first] = Pair.second; + return Symbols.release(); + }(); + this->SymbolMapping = Symbols; + } else if (Language.C11) { + static const auto *Symbols = []() { + auto Symbols = std::make_unique>(); + for (const auto &Pair : CSymbolMap) + (*Symbols)[Pair.first] = Pair.second; + return Symbols.release(); + }(); + this->SymbolMapping = Symbols; + } + + static const auto *SuffixMap = new PathSuffixMap(buildSuffixMap()); + SuffixHeaderMapping = &SuffixMap->SuffixHeaderMapping; + MaxSuffixComponents = SuffixMap->MaxSuffixComponents; } } // namespace clangd 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 @@ -141,7 +141,7 @@ std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { CI.getPreprocessor().addCommentHandler(PragmaHandler.get()); - addSystemHeadersMapping(Includes.get(), CI.getLangOpts()); + Includes->addSystemHeadersMapping(CI.getLangOpts()); if (IncludeGraphCallback != nullptr) CI.getPreprocessor().addPPCallbacks( std::make_unique(CI.getSourceManager(), IG)); 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 @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "index/CanonicalIncludes.h" +#include "clang/Basic/LangOptions.h" #include "gtest/gtest.h" namespace clang { @@ -17,7 +18,7 @@ CanonicalIncludes CI; auto Language = LangOptions(); Language.C11 = true; - addSystemHeadersMapping(&CI, Language); + CI.addSystemHeadersMapping(Language); // Usual standard library symbols are mapped correctly. EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf")); } @@ -26,7 +27,7 @@ CanonicalIncludes CI; auto Language = LangOptions(); Language.CPlusPlus = true; - addSystemHeadersMapping(&CI, Language); + CI.addSystemHeadersMapping(Language); // Usual standard library symbols are mapped correctly. EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector")); @@ -54,19 +55,32 @@ TEST(CanonicalIncludesTest, SymbolMapping) { // As used for standard library. CanonicalIncludes CI; - CI.addSymbolMapping("some::symbol", ""); + LangOptions Language; + Language.CPlusPlus = true; + // Ensures 'std::vector' is mapped to ''. + CI.addSystemHeadersMapping(Language); - EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol")); + EXPECT_EQ("", CI.mapHeader("foo/bar", "std::vector")); EXPECT_EQ("foo/bar", CI.mapHeader("foo/bar", "other::symbol")); } TEST(CanonicalIncludesTest, Precedence) { CanonicalIncludes CI; CI.addMapping("some/path", ""); - CI.addSymbolMapping("some::symbol", ""); + LangOptions Language; + Language.CPlusPlus = true; + CI.addSystemHeadersMapping(Language); + + // We added a mapping from some/path to . + ASSERT_EQ("", CI.mapHeader("some/path", "")); + // We should have a path from 'bits/stl_vector.h' to ''. + ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h", "")); + // We should also have a symbol mapping from 'std::map' to ''. + ASSERT_EQ("", CI.mapHeader("some/header.h", "std::map")); - // Symbol mapping beats path mapping. - EXPECT_EQ("", CI.mapHeader("some/path", "some::symbol")); + // And the symbol mapping should take precedence over paths mapping. + EXPECT_EQ("", CI.mapHeader("bits/stl_vector.h", "std::map")); + EXPECT_EQ("", CI.mapHeader("some/path", "std::map")); } } // namespace 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 @@ -974,7 +974,7 @@ CanonicalIncludes Includes; auto Language = LangOptions(); Language.CPlusPlus = true; - addSystemHeadersMapping(&Includes, Language); + Includes.addSystemHeadersMapping(Language); CollectorOpts.Includes = &Includes; runSymbolCollector("namespace std { class string {}; }", /*Main=*/""); EXPECT_THAT(Symbols,