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

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

llvm-svn: 341534

clang-tools-extra/clangd/AST.cpp

@@ -61,5 +61,16 @@ llvm::Optional<SymbolID> getSymbolID(const Decl *D) {
   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/clangd/AST.h

@@ -37,6 +37,17 @@ 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
 

clang-tools-extra/clangd/CodeComplete.cpp

@@ -505,14 +505,15 @@ private:
 };
 
 // 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;
   }
@@ -1435,7 +1436,8 @@ private:
     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);

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

@@ -385,18 +385,16 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
         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);
@@ -445,11 +443,9 @@ void SymbolCollector::finish() {
   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);
     }
   }
 

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

@@ -1882,6 +1882,17 @@ TEST(CompletionTest, NoInsertIncludeIfOnePresent) {
           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