This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0df197516b69: [clangd] Value initialize SymbolIDs (authored
by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90397/new/
https://reviews.llvm.org/D90397
Files:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/HeaderSourceSwitch.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolID.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 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);
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ 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;
}
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ 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<std::string> 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<std::vector<Range>> AffectedFiles;
Index: clang-tools-extra/clangd/index/SymbolID.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolID.h
+++ 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 @@
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);
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ 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<bool>(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<unsigned>(index::SymbolRole::Definition))
addDefinition(*OriginalDecl, *BasicSymbol);
@@ -422,7 +422,7 @@
// 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 @@
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);
}
}
}
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ 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<SymbolID> ID = getSymbolID(CXXRD))
- fillSubTypes(*ID, *Result->children, Index, ResolveLevels, TUPath);
+ if (auto ID = getSymbolID(CXXRD))
+ fillSubTypes(ID, *Result->children, Index, ResolveLevels, TUPath);
}
}
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1053,7 +1053,7 @@
/// (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 &);
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ 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);
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -133,7 +133,7 @@
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;
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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);
});
Index: clang-tools-extra/clangd/HeaderSourceSwitch.cpp
===================================================================
--- clang-tools-extra/clangd/HeaderSourceSwitch.cpp
+++ 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<int> Candidates; // Target path => score.
auto AwardTarget = [&](const char *TargetURI) {
Index: clang-tools-extra/clangd/CollectMacros.cpp
===================================================================
--- clang-tools-extra/clangd/CollectMacros.cpp
+++ 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);
}
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -503,20 +503,19 @@
};
// 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 @@
[&](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;
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ 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<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.
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -282,21 +282,20 @@
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);
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
