[clangd] Use name of Macro to compute its SymbolID.

[usaxena95]

Summary

We use the name from the IdentifierInfo of the Macro to compute its
SymbolID. It is better to just take the Name as a parameter to avoid
storing the IdentifierInfo whenever we need the SymbolID for the Macro.

[hokein]

there are some unrelated changes in this patch (probably trigger by your editor setting?), though they are trivial, I would avoid these changes in a same patch.

clang-tools-extra/clangd/AST.cpp

238

hmm, this is a unrelated change.

clang-tools-extra/clangd/CodeComplete.cpp

1769

same here.

[usaxena95]

Addressed comments.

[ilya-biryukov]

Does that mean we identify each macro merely by its name?
It's not uncommon to have multiple #define for the same name, meaning completely different things.

If find references shows all of those, it's not very useful...

[hokein]

In D69937#1736980, @ilya-biryukov wrote:

Does that mean we identify each macro merely by its name?
It's not uncommon to have multiple #define for the same name, meaning completely different things.

If find references shows all of those, it's not very useful...

no, I would say this patch is NFC, we still construct macro symbol id with macro name and definition location.

[usaxena95]

We actually use both the name and the source location of the macro to calculate its ID.
I see that the subject of the patch might suggest otherwise.
This is a trivial change which just changes the params of the function so that users don't have to carry the IdentifierInfo when we just want the name out of it.

We use clang::index::generateUSRForMacro(StringRef MacroName, SourceLocation Loc, const SourceManager &SM, SmallVectorImpl<char> &Buf) to generate the SymbolID.

[ilya-biryukov]

Sorry for not reading through and assuming the wrong thing from the title. Thanks for the explanation! LG

[usaxena95]

• [clangd] Store xref for Macros in ParsedAST.

[usaxena95]

Hopefully reverting unintended changes.

clang-tools-extra/clangd/AST.h

 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 
 #include "index/SymbolID.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/MacroInfo.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 class SourceManager;
 class Decl;
 
 namespace clangd {
 
 /// Returns true if the declaration is considered implementation detail based on

 
 /// 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,
+llvm::Optional<SymbolID> getSymbolID(const llvm::StringRef MacroName,
                                      const MacroInfo *MI,
                                      const SourceManager &SM);
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like annoymous/inline namespace.
 std::string printType(const QualType QT, const DeclContext & Context);
 
 /// Try to shorten the OriginalName by removing namespaces from the left of

clang-tools-extra/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,
+llvm::Optional<SymbolID> getSymbolID(const llvm::StringRef MacroName,
                                      const MacroInfo *MI,
                                      const SourceManager &SM) {
   if (MI == nullptr)
     return None;
   llvm::SmallString<128> USR;
-  if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR))
+  if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
     return None;
   return SymbolID(USR);
 }
 
 std::string shortenNamespace(const llvm::StringRef OriginalName,
                              const llvm::StringRef CurrentNamespace) {
   llvm::SmallVector<llvm::StringRef, 8> OriginalParts;
   llvm::SmallVector<llvm::StringRef, 8> CurrentParts;
std::string shortenNamespace(const llvm::StringRef OriginalName,
   }
 
   for (unsigned i = DifferentAt; i < OriginalParts.size(); ++i) {
     Result.push_back(OriginalParts[i]);
   }
   return join(Result, "::");
 }
 
 std::string printType(const QualType QT, const DeclContext & Context){
   PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
   PP.SuppressUnwrittenScope = 1;
   PP.SuppressTagKeyword = 1;
   return shortenNamespace(
       QT.getAsString(PP),
       printNamespaceScope(Context) );
 }
 
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/CodeComplete.cpp

 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:
-    return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM);
+    return clang::clangd::getSymbolID(R.Macro->getName(), 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.
SignatureHelp signatureHelp(PathRef FileName,
   clang::CodeCompleteOptions Options;
   Options.IncludeGlobals = false;
   Options.IncludeMacros = false;
   Options.IncludeCodePatterns = false;
   Options.IncludeBriefComments = false;
   IncludeStructure PreambleInclusions; // Unused for signatureHelp
   semaCodeComplete(
       std::make_unique<SignatureHelpCollector>(Options, Index, Result),
       Options,
       {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});
   return Result;
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
   auto InTopLevelScope = [](const NamedDecl &ND) {
     switch (ND.getDeclContext()->getDeclKind()) {
     case Decl::TranslationUnit:

clang-tools-extra/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;
 
-  auto ID = getSymbolID(*Name, MI, SM);
+  auto ID = getSymbolID(Name->getName(), MI, SM);
   if (!ID)
     return true;
 
   // Only collect one instance in case there are multiple.
   if (Symbols.find(*ID) != nullptr)
     return true;
 
   Symbol S;
if (auto ID = getSymbolID(ND)) {
       IncRef(*ID);
     }
   }
   if (Opts.CollectMacro) {
     assert(PP);
     // First, drop header guards. We can't identify these until EOF.
     for (const IdentifierInfo *II : IndexedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-        if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+        if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
           if (MI->isUsedForHeaderGuard())
             Symbols.erase(*ID);
     }
     // Now increment refcounts.
     for (const IdentifierInfo *II : ReferencedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-        if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+        if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
           IncRef(*ID);
     }
   }
 
   // Fill in IncludeHeaders.
   // We delay this until end of TU so header guards are all resolved.
   // Symbols in slabs aren' mutable, so insert() has to walk all the strings :-(
   llvm::SmallString<256> QName;