[clangd] Set SymbolID for sema macros so that they can be merged with index macros.

[ioeric]

• Set name for the test symbol.

[sammccall]

clangd/AST.h

41 ↗

(On Diff #164055)

maybe add a comment about the semantics of macro symbol IDs - particularly that they depend on the exact file offset.

We could change these semantics in future by reimplementing this function, there's no reason we have to use USRs for everything, and macros are really simple.

clangd/index/SymbolCollector.cpp

389 ↗

(On Diff #164055)

this is now dead

452 ↗

(On Diff #164055)

update this to use getSymbolID

[ioeric]

• improve comment; remove dead code.

clang-tools-extra/trunk/clangd/AST.h

 
 /// Returns the qualified name of ND. The scope doesn't contain unwritten scopes
 /// like inline namespaces.
 std::string printQualifiedName(const NamedDecl &ND);
 
 /// Gets the symbol ID for a declaration, if possible.
 llvm::Optional<SymbolID> getSymbolID(const Decl *D);
 
+/// Gets the symbol ID for a macro, if possible.
+/// Currently, this is an encoded USR of the macro, which incorporates macro
+/// locations (e.g. file name, offset in file).
+/// FIXME: the USR semantics might not be stable enough as the ID for index
+/// macro (e.g. a change in definition offset can result in a different USR). We
+/// could change these semantics in the future by reimplementing this funcure
+/// (e.g. avoid USR for macros).
+llvm::Optional<SymbolID> getSymbolID(const IdentifierInfo &II,
+                                     const MacroInfo *MI,
+                                     const SourceManager &SM);
+
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_

clang-tools-extra/trunk/clangd/AST.cpp

 
 llvm::Optional<SymbolID> getSymbolID(const Decl *D) {
   llvm::SmallString<128> USR;
   if (index::generateUSRForDecl(D, USR))
     return None;
   return SymbolID(USR);
 }
 
+llvm::Optional<SymbolID> getSymbolID(const IdentifierInfo &II,
+                                     const MacroInfo *MI,
+                                     const SourceManager &SM) {
+  if (MI == nullptr)
+    return None;
+  llvm::SmallString<128> USR;
+  if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR))
+    return None;
+  return SymbolID(USR);
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/CodeComplete.cpp

private:
   ASTContext &ASTCtx;
   CodeCompletion Completion;
   SmallVector<BundledEntry, 1> Bundled;
   bool ExtractDocumentation;
   bool EnableFunctionArgSnippets;
 };
 
 // Determine the symbol ID for a Sema code completion result, if possible.
-llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R) {
+llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R,
+                                     const SourceManager &SM) {
   switch (R.Kind) {
   case CodeCompletionResult::RK_Declaration:
   case CodeCompletionResult::RK_Pattern: {
     return clang::clangd::getSymbolID(R.Declaration);
   }
   case CodeCompletionResult::RK_Macro:
-    // FIXME: Macros do have USRs, but the CCR doesn't contain enough info.
+    return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM);
   case CodeCompletionResult::RK_Keyword:
     return None;
   }
   llvm_unreachable("unknown CodeCompletionResult kind");
 }
 
 // Scopes of the paritial identifier we're trying to complete.
 // It is used when we query the index for more completion results.
auto AddToBundles = [&](const CodeCompletionResult *SemaResult,
       } else {
         Bundles.emplace_back();
         Bundles.back().push_back(std::move(C));
       }
     };
     llvm::DenseSet<const Symbol *> UsedIndexResults;
     auto CorrespondingIndexResult =
         [&](const CodeCompletionResult &SemaResult) -> const Symbol * {
-      if (auto SymID = getSymbolID(SemaResult)) {
+      if (auto SymID =
+              getSymbolID(SemaResult, Recorder->CCSema->getSourceManager())) {
         auto I = IndexResults.find(*SymID);
         if (I != IndexResults.end()) {
           UsedIndexResults.insert(&*I);
           return &*I;
         }
       }
       return nullptr;
     };

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

if (Opts.CountReferences &&
       SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedMacros.insert(Name);
   // Don't continue indexing if this is a mere reference.
   // FIXME: remove macro with ID if it is undefined.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
     return true;
 
-  llvm::SmallString<128> USR;
-  if (index::generateUSRForMacro(Name->getName(), MI->getDefinitionLoc(), SM,
-                                 USR))
+  auto ID = getSymbolID(*Name, MI, SM);
+  if (!ID)
     return true;
-  SymbolID ID(USR);
 
   // Only collect one instance in case there are multiple.
-  if (Symbols.find(ID) != nullptr)
+  if (Symbols.find(*ID) != nullptr)
     return true;
 
   Symbol S;
-  S.ID = std::move(ID);
+  S.ID = std::move(*ID);
   S.Name = Name->getName();
   S.IsIndexedForCodeCompletion = true;
   S.SymInfo = index::getSymbolInfoForMacro(*MI);
   std::string FileURI;
   if (auto DeclLoc = getTokenLocation(MI->getDefinitionLoc(), SM, Opts,
                                       PP->getLangOpts(), FileURI))
     S.CanonicalDeclaration = *DeclLoc;
 
void SymbolCollector::finish() {
   for (const NamedDecl *ND : ReferencedDecls) {
     if (auto ID = getSymbolID(ND)) {
       IncRef(*ID);
     }
   }
   if (Opts.CollectMacro) {
     assert(PP);
     for (const IdentifierInfo *II : ReferencedMacros) {
-      llvm::SmallString<128> USR;
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-        if (!index::generateUSRForMacro(II->getName(), MI->getDefinitionLoc(),
-                                        PP->getSourceManager(), USR))
-          IncRef(SymbolID(USR));
+        if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+          IncRef(*ID);
     }
   }
 
   const auto &SM = ASTCtx->getSourceManager();
   auto* MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
 
   if (auto MainFileURI = toURI(SM, MainFileEntry->getName(), Opts)) {
     std::string MainURI = *MainFileURI;

clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

TEST(CompletionTest, NoInsertIncludeIfOnePresent) {
   sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
 
   EXPECT_THAT(
       completions(Server, "#include \"foo.h\"\nFun^", {sym}).Completions,
       UnorderedElementsAre(
           AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude()))));
 }
 
+TEST(CompletionTest, MergeMacrosFromIndexAndSema) {
+  Symbol Sym;
+  Sym.Name = "Clangd_Macro_Test";
+  Sym.ID = SymbolID("c:foo.cpp@8@macro@Clangd_Macro_Test");
+  Sym.SymInfo.Kind = index::SymbolKind::Macro;
+  Sym.IsIndexedForCodeCompletion = true;
+  EXPECT_THAT(completions("#define Clangd_Macro_Test\nClangd_Macro_T^", {Sym})
+                  .Completions,
+              UnorderedElementsAre(Named("Clangd_Macro_Test")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang