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 @@ -90,6 +90,8 @@ static const auto *Symbols = new llvm::StringMap({ #define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name, #Header}, #include "StdSymbolMap.inc" + // There are two std::move()s, this is by far the most common. + SYMBOL(move, std::, ) #undef SYMBOL }); StdSymbolMapping = Symbols; 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 @@ -131,7 +131,7 @@ void processRelations(const NamedDecl &ND, const SymbolID &ID, ArrayRef Relations); - llvm::Optional getIncludeHeader(llvm::StringRef QName, FileID); + llvm::Optional getIncludeHeader(const Symbol &S, FileID); bool isSelfContainedHeader(FileID); // Heuristically headers that only want to be included via an umbrella. static bool isDontIncludeMeHeader(llvm::StringRef); 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 @@ -557,11 +557,9 @@ llvm::SmallString<256> QName; for (const auto &Entry : IncludeFiles) if (const Symbol *S = Symbols.find(Entry.first)) { - QName = S->Scope; - QName.append(S->Name); - if (auto Header = getIncludeHeader(QName, Entry.second)) { + if (auto Header = getIncludeHeader(*S, Entry.second)) { Symbol NewSym = *S; - NewSym.IncludeHeaders.push_back({*Header, 1}); + NewSym.IncludeHeaders.push_back({std::move(*Header), 1}); Symbols.insert(NewSym); } } @@ -736,8 +734,8 @@ /// Gets a canonical include (URI of the header or
or "header") for /// header of \p FID (which should usually be the *expansion* file). /// Returns None if includes should not be inserted for this file. -llvm::Optional -SymbolCollector::getIncludeHeader(llvm::StringRef QName, FileID FID) { +llvm::Optional SymbolCollector::getIncludeHeader(const Symbol &S, + FileID FID) { const SourceManager &SM = ASTCtx->getSourceManager(); const FileEntry *FE = SM.getFileEntryForID(FID); if (!FE || FE->getName().empty()) @@ -746,10 +744,18 @@ // If a file is mapped by canonical headers, use that mapping, regardless // of whether it's an otherwise-good header (header guards etc). if (Opts.Includes) { + llvm::SmallString<256> QName = S.Scope; + QName.append(S.Name); llvm::StringRef Canonical = Opts.Includes->mapHeader(Filename, QName); // If we had a mapping, always use it. - if (Canonical.startswith("<") || Canonical.startswith("\"")) + if (Canonical.startswith("<") || Canonical.startswith("\"")) { + // Hack: there are two std::move() overloads from different headers. + // CanonicalIncludes returns the common one-arg one from . + if (Canonical == "" && S.Name == "move" && + S.Signature.contains(',')) + Canonical = ""; return Canonical.str(); + } if (Canonical != Filename) return toURI(SM, Canonical, Opts); } @@ -757,7 +763,7 @@ // A .inc or .def file is often included into a real header to define // symbols (e.g. LLVM tablegen files). if (Filename.endswith(".inc") || Filename.endswith(".def")) - return getIncludeHeader(QName, SM.getFileID(SM.getIncludeLoc(FID))); + return getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID))); // Conservatively refuse to insert #includes to files without guards. return llvm::None; } 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 @@ -36,9 +36,9 @@ // Usual standard library symbols are mapped correctly. EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector")); EXPECT_EQ("", CI.mapHeader("path/stdio.h", "std::printf")); - // std::move is ambiguous, currently mapped only based on path - EXPECT_EQ("", CI.mapHeader("libstdc++/bits/move.h", "std::move")); - EXPECT_EQ("path/utility.h", CI.mapHeader("path/utility.h", "std::move")); + // std::move is ambiguous, currently always mapped to + EXPECT_EQ("", + CI.mapHeader("libstdc++/bits/stl_algo.h", "std::move")); // Unknown std symbols aren't mapped. EXPECT_EQ("foo/bar.h", CI.mapHeader("foo/bar.h", "std::notathing")); // iosfwd declares some symbols it doesn't own. 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 @@ -1280,10 +1280,27 @@ Language.CPlusPlus = true; Includes.addSystemHeadersMapping(Language); CollectorOpts.Includes = &Includes; - runSymbolCollector("namespace std { class string {}; }", /*Main=*/""); - EXPECT_THAT(Symbols, - Contains(AllOf(QName("std::string"), DeclURI(TestHeaderURI), - IncludeHeader("")))); + runSymbolCollector( + R"cpp( + namespace std { + class string {}; + // Move overloads have special handling. + template T&& move(T&&); + template O move(I, I, O); + } + )cpp", + /*Main=*/""); + for (const auto &S : Symbols) + llvm::errs() << S.Scope << S.Name << " in " << S.IncludeHeaders.size() + << "\n"; + EXPECT_THAT( + Symbols, + UnorderedElementsAre( + QName("std"), + AllOf(QName("std::string"), DeclURI(TestHeaderURI), + IncludeHeader("")), + AllOf(Labeled("move(T &&)"), IncludeHeader("")), + AllOf(Labeled("move(I, I, O)"), IncludeHeader("")))); } TEST_F(SymbolCollectorTest, IWYUPragma) {