[clangd] Disambiguate overloads of std::move for header insertion.

Up until now, we relied on matching the filename.
This depends on unstable details of libstdc++ and doesn't work well on other
stdlibs. Also we'd like to remove it (see D88204).

Differential Revision: https://reviews.llvm.org/D88885

clang-tools-extra/clangd/index/CanonicalIncludes.cpp

@@ -90,6 +90,8 @@ void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) {
     static const auto *Symbols = new llvm::StringMap<llvm::StringRef>({
 #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::, <utility>)
 #undef SYMBOL
     });
     StdSymbolMapping = Symbols;

clang-tools-extra/clangd/index/SymbolCollector.cpp

@@ -557,11 +557,9 @@ void SymbolCollector::finish() {
   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 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
 /// Gets a canonical include (URI of the header or <header> 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<std::string>
-SymbolCollector::getIncludeHeader(llvm::StringRef QName, FileID FID) {
+llvm::Optional<std::string> 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 @@ SymbolCollector::getIncludeHeader(llvm::StringRef QName, FileID FID) {
   // 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 <utility>.
+      if (Canonical == "<utility>" && S.Name == "move" &&
+          S.Signature.contains(','))
+        Canonical = "<algorithm>";
       return Canonical.str();
+    }
     if (Canonical != Filename)
       return toURI(SM, Canonical, Opts);
   }
@@ -757,7 +763,7 @@ SymbolCollector::getIncludeHeader(llvm::StringRef QName, FileID FID) {
     // 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;
   }

clang-tools-extra/clangd/index/SymbolCollector.h

@@ -131,7 +131,7 @@ private:
   void processRelations(const NamedDecl &ND, const SymbolID &ID,
                         ArrayRef<index::SymbolRelation> Relations);
 
-  llvm::Optional<std::string> getIncludeHeader(llvm::StringRef QName, FileID);
+  llvm::Optional<std::string> getIncludeHeader(const Symbol &S, FileID);
   bool isSelfContainedHeader(FileID);
   // Heuristically headers that only want to be included via an umbrella.
   static bool isDontIncludeMeHeader(llvm::StringRef);

clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp

@@ -36,9 +36,9 @@ TEST(CanonicalIncludesTest, CXXStandardLibrary) {
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("<vector>", CI.mapHeader("path/vector.h", "std::vector"));
   EXPECT_EQ("<cstdio>", CI.mapHeader("path/stdio.h", "std::printf"));
-  // std::move is ambiguous, currently mapped only based on path
-  EXPECT_EQ("<utility>", 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 <utility>
+  EXPECT_EQ("<utility>",
+            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.

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

@@ -1280,10 +1280,27 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
   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("<string>"))));
+  runSymbolCollector(
+      R"cpp(
+      namespace std {
+        class string {};
+        // Move overloads have special handling.
+        template <typename T> T&& move(T&&);
+        template <typename I, typename O> 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("<string>")),
+          AllOf(Labeled("move(T &&)"), IncludeHeader("<utility>")),
+          AllOf(Labeled("move(I, I, O)"), IncludeHeader("<algorithm>"))));
 }
 
 TEST_F(SymbolCollectorTest, IWYUPragma) {