[clangd] Value initialize SymbolIDs

We were default initializing SymbolIDs before, which would leave
indeterminate values in underlying std::array.

This patch updates the underlying data initalization to be value-init and adds a
way to check for validness of a SymbolID.

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

clang-tools-extra/clangd/AST.cpp

@@ -282,21 +282,20 @@ std::string printNamespaceScope(const DeclContext &DC) {
   return "";
 }
 
-llvm::Optional<SymbolID> getSymbolID(const Decl *D) {
+SymbolID getSymbolID(const Decl *D) {
   llvm::SmallString<128> USR;
   if (index::generateUSRForDecl(D, USR))
-    return None;
+    return {};
   return SymbolID(USR);
 }
 
-llvm::Optional<SymbolID> getSymbolID(const llvm::StringRef MacroName,
-                                     const MacroInfo *MI,
-                                     const SourceManager &SM) {
+SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
+                     const SourceManager &SM) {
   if (MI == nullptr)
-    return None;
+    return {};
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
-    return None;
+    return {};
   return SymbolID(USR);
 }
 

clang-tools-extra/clangd/AST.h

@@ -64,19 +64,18 @@ std::string printName(const ASTContext &Ctx, const NamedDecl &ND);
 /// string if decl is not a template specialization.
 std::string printTemplateSpecializationArgs(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 declaration. Returned SymbolID might be null.
+SymbolID getSymbolID(const Decl *D);
 
-/// Gets the symbol ID for a macro, if possible.
+/// Gets the symbol ID for a macro. Returned SymbolID might be null.
 /// 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 llvm::StringRef MacroName,
-                                     const MacroInfo *MI,
-                                     const SourceManager &SM);
+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 anonymous/inline namespace.

clang-tools-extra/clangd/CodeComplete.cpp

@@ -503,20 +503,19 @@ private:
 };
 
 // Determine the symbol ID for a Sema code completion result, if possible.
-llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R,
-                                     const SourceManager &SM) {
+SymbolID getSymbolID(const CodeCompletionResult &R, const SourceManager &SM) {
   switch (R.Kind) {
   case CodeCompletionResult::RK_Declaration:
   case CodeCompletionResult::RK_Pattern: {
     // Computing USR caches linkage, which may change after code completion.
     if (hasUnstableLinkage(R.Declaration))
-      return llvm::None;
+      return {};
     return clang::clangd::getSymbolID(R.Declaration);
   }
   case CodeCompletionResult::RK_Macro:
     return clang::clangd::getSymbolID(R.Macro->getName(), R.MacroDefInfo, SM);
   case CodeCompletionResult::RK_Keyword:
-    return None;
+    return {};
   }
   llvm_unreachable("unknown CodeCompletionResult kind");
 }
@@ -1586,7 +1585,7 @@ private:
         [&](const CodeCompletionResult &SemaResult) -> const Symbol * {
       if (auto SymID =
               getSymbolID(SemaResult, Recorder->CCSema->getSourceManager())) {
-        auto I = IndexResults.find(*SymID);
+        auto I = IndexResults.find(SymID);
         if (I != IndexResults.end()) {
           UsedIndexResults.insert(&*I);
           return &*I;

clang-tools-extra/clangd/CollectMacros.cpp

@@ -26,7 +26,7 @@ void CollectMainFileMacros::add(const Token &MacroNameTok,
   auto Range = halfOpenToRange(
       SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
   if (auto SID = getSymbolID(Name, MI, SM))
-    Out.MacroRefs[*SID].push_back(Range);
+    Out.MacroRefs[SID].push_back(Range);
   else
     Out.UnknownMacros.push_back(Range);
 }

clang-tools-extra/clangd/HeaderSourceSwitch.cpp

@@ -80,7 +80,7 @@ llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
   // Find all symbols present in the original file.
   for (const auto *D : getIndexableLocalDecls(AST)) {
     if (auto ID = getSymbolID(D))
-      Request.IDs.insert(*ID);
+      Request.IDs.insert(ID);
   }
   llvm::StringMap<int> Candidates; // Target path => score.
   auto AwardTarget = [&](const char *TargetURI) {

clang-tools-extra/clangd/Hover.cpp

@@ -269,7 +269,7 @@ void enhanceFromIndex(HoverInfo &Hover, const NamedDecl &ND,
   if (!ID)
     return;
   LookupRequest Req;
-  Req.IDs.insert(*ID);
+  Req.IDs.insert(ID);
   Index->lookup(Req, [&](const Symbol &S) {
     Hover.Documentation = std::string(S.Documentation);
   });

clang-tools-extra/clangd/IncludeFixer.cpp

@@ -133,7 +133,7 @@ std::vector<Fix> IncludeFixer::fixIncompleteType(const Type &T) const {
   auto ID = getSymbolID(TD);
   if (!ID)
     return {};
-  llvm::Optional<const SymbolSlab *> Symbols = lookupCached(*ID);
+  llvm::Optional<const SymbolSlab *> Symbols = lookupCached(ID);
   if (!Symbols)
     return {};
   const SymbolSlab &Syms = **Symbols;

clang-tools-extra/clangd/Protocol.cpp

@@ -701,8 +701,8 @@ llvm::json::Value toJSON(const SymbolDetails &P) {
   if (!P.USR.empty())
     Result["usr"] = P.USR;
 
-  if (P.ID.hasValue())
-    Result["id"] = P.ID.getValue().str();
+  if (P.ID)
+    Result["id"] = P.ID.str();
 
   // FIXME: workaround for older gcc/clang
   return std::move(Result);

clang-tools-extra/clangd/Protocol.h

@@ -1053,7 +1053,7 @@ struct SymbolDetails {
   /// (See USRGeneration.h)
   std::string USR;
 
-  llvm::Optional<SymbolID> ID;
+  SymbolID ID;
 };
 llvm::json::Value toJSON(const SymbolDetails &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolDetails &);

clang-tools-extra/clangd/XRefs.cpp

@@ -321,7 +321,7 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
 
     // Record SymbolID for index lookup later.
     if (auto ID = getSymbolID(D))
-      ResultIndex[*ID] = Result.size() - 1;
+      ResultIndex[ID] = Result.size() - 1;
   };
 
   // Emit all symbol locations (declaration or definition) from AST.
@@ -1154,7 +1154,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
     if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
       // Collect macro references from main file.
       const auto &IDToRefs = AST.getMacros().MacroRefs;
-      auto Refs = IDToRefs.find(*MacroSID);
+      auto Refs = IDToRefs.find(MacroSID);
       if (Refs != IDToRefs.end()) {
         for (const auto &Ref : Refs->second) {
           Location Result;
@@ -1163,7 +1163,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
           Results.References.push_back(std::move(Result));
         }
       }
-      Req.IDs.insert(*MacroSID);
+      Req.IDs.insert(MacroSID);
     }
   } else {
     // Handle references to Decls.
@@ -1200,7 +1200,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
         if (D->getParentFunctionOrMethod())
           continue;
         if (auto ID = getSymbolID(D))
-          Req.IDs.insert(*ID);
+          Req.IDs.insert(ID);
       }
     }
   }
@@ -1332,9 +1332,8 @@ declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND) {
   // This allows typeHierarchy/resolve to be used to
   // resolve children of items returned in a previous request
   // for parents.
-  if (auto ID = getSymbolID(&ND)) {
-    THI.data = ID->str();
-  }
+  if (auto ID = getSymbolID(&ND))
+    THI.data = ID.str();
 
   return THI;
 }
@@ -1539,8 +1538,8 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels,
     Result->children.emplace();
 
     if (Index) {
-      if (Optional<SymbolID> ID = getSymbolID(CXXRD))
-        fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath);
+      if (auto ID = getSymbolID(CXXRD))
+        fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath);
     }
   }
 

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

@@ -324,7 +324,7 @@ bool SymbolCollector::handleDeclOccurrence(
   // refs, because the indexing code only populates relations for specific
   // occurrences. For example, RelationBaseOf is only populated for the
   // occurrence inside the base-specifier.
-  processRelations(*ND, *ID, Relations);
+  processRelations(*ND, ID, Relations);
 
   bool CollectRef = static_cast<bool>(Opts.RefFilter & toRefKind(Roles));
   bool IsOnlyRef =
@@ -356,15 +356,15 @@ bool SymbolCollector::handleDeclOccurrence(
   if (!OriginalDecl)
     return true;
 
-  const Symbol *BasicSymbol = Symbols.find(*ID);
+  const Symbol *BasicSymbol = Symbols.find(ID);
   if (isPreferredDeclaration(*OriginalDecl, Roles))
     // If OriginalDecl is preferred, replace/create the existing canonical
     // declaration (e.g. a class forward declaration). There should be at most
     // one duplicate as we expect to see only one preferred declaration per
     // TU, because in practice they are definitions.
-    BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), IsMainFileOnly);
+    BasicSymbol = addDeclaration(*OriginalDecl, std::move(ID), IsMainFileOnly);
   else if (!BasicSymbol || DeclIsCanonical)
-    BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
+    BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly);
 
   if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
     addDefinition(*OriginalDecl, *BasicSymbol);
@@ -422,7 +422,7 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name,
   // Do not store references to main-file macros.
   if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileOnly &&
       (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
-    MacroRefs[*ID].push_back({Loc, Roles});
+    MacroRefs[ID].push_back({Loc, Roles});
 
   // Collect symbols.
   if (!Opts.CollectMacro)
@@ -447,11 +447,11 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name,
     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();
   if (!IsMainFileOnly) {
     S.Flags |= Symbol::IndexedForCodeCompletion;
@@ -507,7 +507,7 @@ void SymbolCollector::processRelations(
     //       in the index and find nothing, but that's a situation they
     //       probably need to handle for other reasons anyways.
     // We currently do (B) because it's simpler.
-    this->Relations.insert(Relation{ID, RelationKind::BaseOf, *ObjectID});
+    this->Relations.insert(Relation{ID, RelationKind::BaseOf, ObjectID});
   }
 }
 
@@ -531,7 +531,7 @@ void SymbolCollector::finish() {
   };
   for (const NamedDecl *ND : ReferencedDecls) {
     if (auto ID = getSymbolID(ND)) {
-      IncRef(*ID);
+      IncRef(ID);
     }
   }
   if (Opts.CollectMacro) {
@@ -541,13 +541,13 @@ void SymbolCollector::finish() {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
           if (MI->isUsedForHeaderGuard())
-            Symbols.erase(*ID);
+            Symbols.erase(ID);
     }
     // Now increment refcounts.
     for (const IdentifierInfo *II : ReferencedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
-          IncRef(*ID);
+          IncRef(ID);
     }
   }
   // Fill in IncludeHeaders.
@@ -624,7 +624,7 @@ void SymbolCollector::finish() {
                             NameKind == DeclarationName::CXXConstructorName;
         bool Spelled = IdentifierToken && IsTargetKind &&
                        Name.getAsString() == IdentifierToken->text(SM);
-        CollectRef(*ID, LocAndRole, Spelled);
+        CollectRef(ID, LocAndRole, Spelled);
       }
     }
   }

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

@@ -15,6 +15,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include <array>
+#include <cstdint>
 #include <string>
 
 namespace clang {
@@ -53,8 +54,11 @@ public:
   std::string str() const;
   static llvm::Expected<SymbolID> fromStr(llvm::StringRef);
 
+  bool isNull() const { return *this == SymbolID(); }
+  explicit operator bool() const { return !isNull(); }
+
 private:
-  std::array<uint8_t, RawSize> HashValue;
+  std::array<uint8_t, RawSize> HashValue{};
 };
 
 llvm::hash_code hash_value(const SymbolID &ID);

clang-tools-extra/clangd/refactor/Rename.cpp

@@ -63,7 +63,7 @@ llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
   // tradeoff. We expect the number of symbol references in the current file
   // is smaller than the limit.
   Req.Limit = 100;
-  Req.IDs.insert(*getSymbolID(&D));
+  Req.IDs.insert(getSymbolID(&D));
   llvm::Optional<std::string> OtherFile;
   Index.refs(Req, [&](const Ref &R) {
     if (OtherFile)
@@ -244,7 +244,7 @@ std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
         return;
       for (const auto *Target : Ref.Targets) {
         auto ID = getSymbolID(Target);
-        if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
+        if (!ID || TargetIDs.find(ID) == TargetIDs.end())
           return;
       }
       Results.push_back(Ref.NameLoc);
@@ -304,7 +304,7 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
                            size_t MaxLimitFiles) {
   trace::Span Tracer("FindOccurrencesOutsideFile");
   RefsRequest RQuest;
-  RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+  RQuest.IDs.insert(getSymbolID(&RenameDecl));
 
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap<std::vector<Range>> AffectedFiles;

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

@@ -96,7 +96,7 @@ TEST(CollectMainFileMacros, SelectedMacros) {
       auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
 
       EXPECT_THAT(ExpectedRefs,
-                  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[*SID]))
+                  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID]))
           << "Annotation=" << I << ", MacroName=" << Macro->Name
           << ", Test = " << Test;
     }

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

@@ -2077,7 +2077,7 @@ TEST(Hover, DocsFromIndex) {
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
   Symbol IndexSym;
-  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.ID = getSymbolID(&findDecl(AST, "X"));
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);