diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -64,19 +64,18 @@ /// 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 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 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. diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -282,21 +282,20 @@ return ""; } -llvm::Optional 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 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); } diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -503,20 +503,19 @@ }; // Determine the symbol ID for a Sema code completion result, if possible. -llvm::Optional 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 @@ [&](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; diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp --- a/clang-tools-extra/clangd/CollectMacros.cpp +++ b/clang-tools-extra/clangd/CollectMacros.cpp @@ -26,7 +26,7 @@ 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); } diff --git a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp --- a/clang-tools-extra/clangd/HeaderSourceSwitch.cpp +++ b/clang-tools-extra/clangd/HeaderSourceSwitch.cpp @@ -80,7 +80,7 @@ // 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 Candidates; // Target path => score. auto AwardTarget = [&](const char *TargetURI) { diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -269,7 +269,7 @@ 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); }); diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp --- a/clang-tools-extra/clangd/IncludeFixer.cpp +++ b/clang-tools-extra/clangd/IncludeFixer.cpp @@ -133,7 +133,7 @@ auto ID = getSymbolID(TD); if (!ID) return {}; - llvm::Optional Symbols = lookupCached(*ID); + llvm::Optional Symbols = lookupCached(ID); if (!Symbols) return {}; const SymbolSlab &Syms = **Symbols; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1053,7 +1053,7 @@ /// (See USRGeneration.h) std::string USR; - llvm::Optional ID; + SymbolID ID; }; llvm::json::Value toJSON(const SymbolDetails &); llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolDetails &); diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -701,8 +701,8 @@ 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); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -321,7 +321,7 @@ // 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 @@ 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 @@ Results.References.push_back(std::move(Result)); } } - Req.IDs.insert(*MacroSID); + Req.IDs.insert(MacroSID); } } else { // Handle references to Decls. @@ -1200,7 +1200,7 @@ if (D->getParentFunctionOrMethod()) continue; if (auto ID = getSymbolID(D)) - Req.IDs.insert(*ID); + Req.IDs.insert(ID); } } } @@ -1332,9 +1332,8 @@ // 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 @@ Result->children.emplace(); if (Index) { - if (Optional ID = getSymbolID(CXXRD)) - fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath); + if (auto ID = getSymbolID(CXXRD)) + fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath); } } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -324,7 +324,7 @@ // 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(Opts.RefFilter & toRefKind(Roles)); bool IsOnlyRef = @@ -356,15 +356,15 @@ 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(index::SymbolRole::Definition)) addDefinition(*OriginalDecl, *BasicSymbol); @@ -422,7 +422,7 @@ // Do not store references to main-file macros. if ((static_cast(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 @@ 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 @@ // 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 @@ }; for (const NamedDecl *ND : ReferencedDecls) { if (auto ID = getSymbolID(ND)) { - IncRef(*ID); + IncRef(ID); } } if (Opts.CollectMacro) { @@ -541,13 +541,13 @@ 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 @@ NameKind == DeclarationName::CXXConstructorName; bool Spelled = IdentifierToken && IsTargetKind && Name.getAsString() == IdentifierToken->text(SM); - CollectRef(*ID, LocAndRole, Spelled); + CollectRef(ID, LocAndRole, Spelled); } } } diff --git a/clang-tools-extra/clangd/index/SymbolID.h b/clang-tools-extra/clangd/index/SymbolID.h --- a/clang-tools-extra/clangd/index/SymbolID.h +++ b/clang-tools-extra/clangd/index/SymbolID.h @@ -15,6 +15,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/raw_ostream.h" #include +#include #include namespace clang { @@ -53,8 +54,11 @@ std::string str() const; static llvm::Expected fromStr(llvm::StringRef); + bool isNull() const { return *this == SymbolID(); } + explicit operator bool() const { return !isNull(); } + private: - std::array HashValue; + std::array HashValue{}; }; llvm::hash_code hash_value(const SymbolID &ID); diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -63,7 +63,7 @@ // 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 OtherFile; Index.refs(Req, [&](const Ref &R) { if (OtherFile) @@ -244,7 +244,7 @@ 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 @@ 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> AffectedFiles; diff --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp --- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp +++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp @@ -96,7 +96,7 @@ 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; } diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -2077,7 +2077,7 @@ 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);