Index: clang-tools-extra/trunk/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ParsedAST.cpp +++ clang-tools-extra/trunk/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()); Index: clang-tools-extra/trunk/clangd/Preamble.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Preamble.cpp +++ clang-tools-extra/trunk/clangd/Preamble.cpp @@ -78,7 +78,7 @@ } void BeforeExecute(CompilerInstance &CI) override { - addSystemHeadersMapping(&CanonIncludes, CI.getLangOpts()); + CanonIncludes.addSystemHeadersMapping(CI.getLangOpts()); SourceMgr = &CI.getSourceManager(); } Index: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h =================================================================== --- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h +++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h @@ -21,6 +21,7 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Regex.h" #include #include @@ -34,37 +35,34 @@ /// 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; - /// Maximum number of path components stored in a key of SuffixHeaderMapping. - /// Used to reduce the number of lookups into SuffixHeaderMapping. - int MaxSuffixComponents = 0; + /// Used only for mapping standard headers. + const llvm::StringMap *StdSuffixHeaderMapping = nullptr; /// A map from fully qualified symbol names to header names. - llvm::StringMap SymbolMapping; + /// Used only for mapping standard symbols. + const llvm::StringMap *StdSymbolMapping = nullptr; }; /// Returns a CommentHandler that parses pragma comment on include files to @@ -76,17 +74,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 Index: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp +++ clang-tools-extra/trunk/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,40 @@ 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; -} +/// 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(llvm::StringRef Header, llvm::StringRef QualifiedName) const { assert(!Header.empty()); - auto SE = SymbolMapping.find(QualifiedName); - if (SE != SymbolMapping.end()) - return SE->second; + if (StdSymbolMapping) { + auto SE = StdSymbolMapping->find(QualifiedName); + if (SE != StdSymbolMapping->end()) + return SE->second; + } auto MapIt = FullPathMapping.find(Header); if (MapIt != FullPathMapping.end()) return MapIt->second; + if (!StdSuffixHeaderMapping) + 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 = StdSuffixHeaderMapping->find(SubPath); + if (MappingIt != StdSuffixHeaderMapping->end()) return MappingIt->second; } return Header; @@ -86,29 +84,27 @@ 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 - }; +void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) { if (Language.CPlusPlus) { - for (const auto &Pair : SymbolMap) - Includes->addSymbolMapping(Pair.first, Pair.second); + static const auto *Symbols = new llvm::StringMap({ +#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name, #Header}, +#include "StdSymbolMap.inc" +#undef SYMBOL + }); + StdSymbolMapping = Symbols; } else if (Language.C11) { - for (const auto &Pair : CSymbolMap) - Includes->addSymbolMapping(Pair.first, Pair.second); + static const auto *CSymbols = new llvm::StringMap({ +#define SYMBOL(Name, NameSpace, Header) {#Name, #Header}, +#include "CSymbolMap.inc" +#undef SYMBOL + }); + StdSymbolMapping = CSymbols; } + // 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) - static constexpr std::pair SystemHeaderMap[] = { + static const auto *SystemHeaderMap = new llvm::StringMap({ {"include/__stddef_max_align_t.h", ""}, {"include/__wmmintrin_aes.h", ""}, {"include/__wmmintrin_pclmul.h", ""}, @@ -760,9 +756,20 @@ {"bits/waitflags.h", ""}, {"bits/waitstatus.h", ""}, {"bits/xtitypes.h", ""}, - }; - for (const auto &Pair : SystemHeaderMap) - Includes->addPathSuffixMapping(Pair.first, Pair.second); + }); + // Check MaxSuffixComponents constant is correct. + assert(llvm::all_of(SystemHeaderMap->keys(), [](llvm::StringRef Path) { + return std::distance(llvm::sys::path::begin(Path), + llvm::sys::path::end(Path)) <= MaxSuffixComponents; + })); + // ... and precise. + assert(llvm::find_if(SystemHeaderMap->keys(), [](llvm::StringRef Path) { + return std::distance(llvm::sys::path::begin(Path), + llvm::sys::path::end(Path)) == + MaxSuffixComponents; + }) != SystemHeaderMap->keys().end()); + + StdSuffixHeaderMapping = SystemHeaderMap; } } // namespace clangd Index: clang-tools-extra/trunk/clangd/index/IndexAction.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/IndexAction.cpp +++ clang-tools-extra/trunk/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)); Index: clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp +++ clang-tools-extra/trunk/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); - // Symbol mapping beats path mapping. - EXPECT_EQ("", CI.mapHeader("some/path", "some::symbol")); + // 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")); + + // 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 Index: clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/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,