[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

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

this is now dead

452

update this to use getSymbolID

[ioeric]

• improve comment; remove dead code.

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.
+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_

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

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;
     };

clangd/index/SymbolCollector.cpp

if (Opts.CountReferences &&
     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))
     return true;
-  SymbolID ID(USR);
+  auto ID = getSymbolID(*Name, MI, SM);
+  if (!ID)
+    return true;
 
   // 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;
 
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));
     }
   }
 
   const auto &SM = ASTCtx->getSourceManager();
   auto* MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
 

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.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