Skip to content

Commit 343ecb0

Browse files
varungandhi-srcvarungandhi-src
authored
refactor: Use separate types for symbol names (#344)
1 parent 5c41a0e commit 343ecb0

File tree

6 files changed

+188
-152
lines changed

6 files changed

+188
-152
lines changed

‎indexer/Indexer.cc

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "indexer/Path.h"
3737
#include "indexer/ScipExtras.h"
3838
#include "indexer/SymbolFormatter.h"
39+
#include "indexer/SymbolName.h"
3940
#include "indexer/Tracing.h"
4041

4142
namespace scip_clang {
@@ -105,7 +106,8 @@ void FileLocalMacroOccurrence::emitOccurrence(SymbolFormatter &symbolFormatter,
105106
break;
106107
}
107108
this->range.addToOccurrence(occ);
108-
auto name = symbolFormatter.getMacroSymbol(this->defInfo->getDefinitionLoc());
109+
auto name =
110+
symbolFormatter.getMacroSymbol(this->defInfo->getDefinitionLoc()).value;
109111
occ.set_symbol(name.data(), name.size());
110112
}
111113

@@ -126,7 +128,8 @@ std::strong_ordering operator<=>(const NonFileBasedMacro &m1,
126128
void NonFileBasedMacro::emitSymbolInformation(
127129
SymbolFormatter &symbolFormatter,
128130
scip::SymbolInformation &symbolInfo) const {
129-
auto name = symbolFormatter.getMacroSymbol(this->defInfo->getDefinitionLoc());
131+
auto name =
132+
symbolFormatter.getMacroSymbol(this->defInfo->getDefinitionLoc()).value;
130133
symbolInfo.set_symbol(name.data(), name.size());
131134
}
132135

@@ -465,7 +468,7 @@ void TuIndexer::saveFunctionDecl(const clang::FunctionDecl &functionDecl) {
465468
if (!optSymbol.has_value()) {
466469
return;
467470
}
468-
std::string_view symbol = optSymbol.value();
471+
auto symbol = optSymbol.value();
469472

470473
if (functionDecl.isPure() || functionDecl.isThisDeclarationADefinition()) {
471474
scip::SymbolInformation symbolInfo{};
@@ -476,8 +479,8 @@ void TuIndexer::saveFunctionDecl(const clang::FunctionDecl &functionDecl) {
476479
if (auto optOverridenSymbol =
477480
this->symbolFormatter.getFunctionSymbol(*overridenMethodDecl)) {
478481
scip::Relationship rel{};
479-
rel.set_symbol(optOverridenSymbol->data(),
480-
optOverridenSymbol->size());
482+
auto symbol = optOverridenSymbol->value;
483+
rel.set_symbol(symbol.data(), symbol.size());
481484
rel.set_is_implementation(true);
482485
rel.set_is_reference(true);
483486
*symbolInfo.add_relationships() = std::move(rel);
@@ -511,7 +514,7 @@ void TuIndexer::saveNamespaceDecl(const clang::NamespaceDecl &namespaceDecl) {
511514
if (!optSymbol.has_value()) {
512515
return;
513516
}
514-
std::string_view symbol = optSymbol.value();
517+
auto symbol = optSymbol.value();
515518

516519
// getLocation():
517520
// - for anonymous namespaces, returns the location of the opening brace {
@@ -674,7 +677,8 @@ void TuIndexer::saveTagDecl(const clang::TagDecl &tagDecl) {
674677
continue;
675678
}
676679
scip::Relationship rel{};
677-
rel.set_symbol(optRelatedSymbol->data(), optRelatedSymbol->size());
680+
auto symbol = optRelatedSymbol->value;
681+
rel.set_symbol(symbol.data(), symbol.size());
678682
rel.set_is_implementation(true);
679683
*symbolInfo.add_relationships() = std::move(rel);
680684
}
@@ -719,7 +723,7 @@ void TuIndexer::saveTemplateSpecializationTypeLoc(
719723
switch (templateName.getKind()) {
720724
case Kind::Template: {
721725
auto *templateDecl = templateName.getAsTemplateDecl();
722-
std::optional<std::string_view> optSymbol;
726+
std::optional<SymbolNameRef> optSymbol;
723727
if (auto *classTemplateDecl =
724728
llvm::dyn_cast<clang::ClassTemplateDecl>(templateDecl)) {
725729
optSymbol = this->symbolFormatter.getRecordSymbol(
@@ -934,9 +938,9 @@ void TuIndexer::emitDocumentOccurrencesAndSymbols(
934938
}
935939
extractTransform(
936940
std::move(doc.symbolInfos), deterministic,
937-
absl::FunctionRef<void(std::string_view &&, scip::SymbolInformation &&)>(
938-
[&](auto &&symbolName, auto &&symInfo) {
939-
symInfo.set_symbol(symbolName.data(), symbolName.size());
941+
absl::FunctionRef<void(SymbolNameRef &&, scip::SymbolInformation &&)>(
942+
[&](auto &&symbol, auto &&symInfo) {
943+
symInfo.set_symbol(symbol.value.data(), symbol.value.size());
940944
*scipDocument.add_symbols() = std::move(symInfo);
941945
}));
942946
}
@@ -947,9 +951,9 @@ void TuIndexer::emitExternalSymbols(bool deterministic,
947951
this->externalSymbols.size());
948952
scip_clang::extractTransform(
949953
std::move(this->externalSymbols), deterministic,
950-
absl::FunctionRef<void(std::string_view &&, scip::SymbolInformation &&)>(
954+
absl::FunctionRef<void(SymbolNameRef &&, scip::SymbolInformation &&)>(
951955
[&](auto &&symbol, auto &&symbolInfo) {
952-
symbolInfo.set_symbol(symbol.data(), symbol.size());
956+
symbolInfo.set_symbol(symbol.value.data(), symbol.value.size());
953957
*indexShard.add_external_symbols() = std::move(symbolInfo);
954958
}));
955959
}
@@ -960,11 +964,11 @@ void TuIndexer::emitForwardDeclarations(bool deterministic,
960964
this->forwardDeclarations.size());
961965
scip_clang::extractTransform(
962966
std::move(this->forwardDeclarations), deterministic,
963-
absl::FunctionRef<void(std::string_view &&, DocComment &&)>(
967+
absl::FunctionRef<void(SymbolNameRef &&, DocComment &&)>(
964968
[&](auto &&symbol, auto &&docComment) {
965969
scip::SymbolInformation symbolInfo{};
966970
docComment.addTo(symbolInfo);
967-
symbolInfo.set_symbol(symbol.data(), symbol.size());
971+
symbolInfo.set_symbol(symbol.value.data(), symbol.value.size());
968972
// Add a forward declaration SymbolRole here
969973
// once https://github.com/sourcegraph/scip/issues/131 is fixed.
970974
*forwardDeclIndex.add_external_symbols() = std::move(symbolInfo);
@@ -982,7 +986,7 @@ TuIndexer::getTokenExpansionRange(
982986
{startExpansionLoc, endLoc});
983987
}
984988

985-
void TuIndexer::saveForwardDeclaration(std::string_view symbol,
989+
void TuIndexer::saveForwardDeclaration(SymbolNameRef symbol,
986990
clang::SourceLocation loc,
987991
DocComment &&docComment) {
988992
this->saveReference(symbol, loc);
@@ -995,8 +999,8 @@ void TuIndexer::saveForwardDeclaration(std::string_view symbol,
995999
return;
9961000
}
9971001

998-
void TuIndexer::saveReference(std::string_view symbol,
999-
clang::SourceLocation loc, int32_t extraRoles) {
1002+
void TuIndexer::saveReference(SymbolNameRef symbol, clang::SourceLocation loc,
1003+
int32_t extraRoles) {
10001004
auto expansionLoc = this->sourceManager.getExpansionLoc(loc);
10011005
auto fileId = this->sourceManager.getFileID(expansionLoc);
10021006
auto optStableFileId = this->fileMetadataMap.getStableFileId(fileId);
@@ -1009,7 +1013,7 @@ void TuIndexer::saveReference(std::string_view symbol,
10091013
}
10101014

10111015
void TuIndexer::saveDefinition(
1012-
std::string_view symbol, clang::SourceLocation loc,
1016+
SymbolNameRef symbol, clang::SourceLocation loc,
10131017
std::optional<scip::SymbolInformation> &&optSymbolInfo,
10141018
int32_t extraRoles) {
10151019
auto expansionLoc = this->sourceManager.getExpansionLoc(loc);
@@ -1032,7 +1036,7 @@ void TuIndexer::saveDefinition(
10321036
}
10331037
}
10341038

1035-
void TuIndexer::saveExternalSymbol(std::string_view symbol,
1039+
void TuIndexer::saveExternalSymbol(SymbolNameRef symbol,
10361040
scip::SymbolInformation &&symbolInfo) {
10371041
auto [it, inserted] =
10381042
this->externalSymbols.emplace(symbol, std::move(symbolInfo));
@@ -1043,20 +1047,20 @@ void TuIndexer::saveExternalSymbol(std::string_view symbol,
10431047
}
10441048
}
10451049

1046-
PartialDocument &TuIndexer::saveOccurrence(std::string_view symbol,
1050+
PartialDocument &TuIndexer::saveOccurrence(SymbolNameRef symbol,
10471051
clang::SourceLocation expansionLoc,
10481052
int32_t allRoles) {
10491053
auto [range, fileId] = this->getTokenExpansionRange(expansionLoc);
10501054
return this->saveOccurrenceImpl(symbol, range, fileId, allRoles);
10511055
}
10521056

1053-
PartialDocument &TuIndexer::saveOccurrenceImpl(std::string_view symbol,
1057+
PartialDocument &TuIndexer::saveOccurrenceImpl(SymbolNameRef symbol,
10541058
FileLocalSourceRange range,
10551059
clang::FileID fileId,
10561060
int32_t allRoles) {
10571061
scip::Occurrence occ;
10581062
range.addToOccurrence(occ);
1059-
occ.set_symbol(symbol.data(), symbol.size());
1063+
occ.set_symbol(symbol.value.data(), symbol.value.size());
10601064
occ.set_symbol_roles(allRoles);
10611065
auto &doc = this->documentMap[{fileId}];
10621066
doc.occurrences.emplace_back(scip::OccurrenceExt{std::move(occ)});

‎indexer/Indexer.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "indexer/LlvmAdapter.h"
2525
#include "indexer/Path.h"
2626
#include "indexer/ScipExtras.h"
27+
#include "indexer/SymbolName.h"
2728

2829
namespace clang {
2930
#define FORWARD_DECLARE(ExprName) class ExprName##Expr;
@@ -224,7 +225,7 @@ struct PartialDocument {
224225
std::vector<scip::OccurrenceExt> occurrences;
225226
// Keyed by the symbol name. The symbol name is not set on the
226227
// SymbolInformation value to avoid redundant allocations.
227-
absl::flat_hash_map<std::string_view, scip::SymbolInformation> symbolInfos;
228+
absl::flat_hash_map<scip::SymbolNameRef, scip::SymbolInformation> symbolInfos;
228229
};
229230

230231
class DocComment {
@@ -253,10 +254,9 @@ class TuIndexer final {
253254
documentMap;
254255
FileMetadataMap &fileMetadataMap;
255256

256-
absl::flat_hash_map</*name*/ std::string_view, scip::SymbolInformation>
257-
externalSymbols;
257+
absl::flat_hash_map<SymbolNameRef, scip::SymbolInformation> externalSymbols;
258258

259-
absl::flat_hash_map<std::string_view, DocComment> forwardDeclarations;
259+
absl::flat_hash_map<SymbolNameRef, DocComment> forwardDeclarations;
260260

261261
public:
262262
TuIndexer(const clang::SourceManager &, const clang::LangOptions &,
@@ -310,10 +310,10 @@ class TuIndexer final {
310310
/// Helper method for recording forward declarations.
311311
///
312312
/// Prefer this over \c saveReference or \c saveOccurrence.
313-
void saveForwardDeclaration(std::string_view symbol,
314-
clang::SourceLocation loc, DocComment &&);
313+
void saveForwardDeclaration(SymbolNameRef symbol, clang::SourceLocation loc,
314+
DocComment &&);
315315

316-
void saveReference(std::string_view symbol, clang::SourceLocation loc,
316+
void saveReference(SymbolNameRef symbol, clang::SourceLocation loc,
317317
int32_t extraRoles = 0);
318318

319319
/// Helper method for recording a \c scip::Occurrence and a
@@ -322,12 +322,12 @@ class TuIndexer final {
322322
/// Setting the symbol name on \param symbolInfo is not necessary.
323323
///
324324
/// For local variables, \param symbolInfo should be \c std::nullopt.
325-
void saveDefinition(std::string_view symbol, clang::SourceLocation loc,
325+
void saveDefinition(SymbolNameRef symbol, clang::SourceLocation loc,
326326
std::optional<scip::SymbolInformation> &&symbolInfo,
327327
int32_t extraRoles = 0);
328328

329329
/// Only for use inside \c saveDefinition.
330-
void saveExternalSymbol(std::string_view symbol, scip::SymbolInformation &&);
330+
void saveExternalSymbol(SymbolNameRef symbol, scip::SymbolInformation &&);
331331

332332
/// Lower-level method for only saving a Occurrence.
333333
///
@@ -337,11 +337,11 @@ class TuIndexer final {
337337
///
338338
/// This method should not be called for occurrences in external files,
339339
/// since SCIP only tracks SymbolInformation values in external code.
340-
PartialDocument &saveOccurrence(std::string_view symbol,
340+
PartialDocument &saveOccurrence(SymbolNameRef symbol,
341341
clang::SourceLocation loc,
342342
int32_t allRoles = 0);
343343

344-
PartialDocument &saveOccurrenceImpl(std::string_view symbol,
344+
PartialDocument &saveOccurrenceImpl(SymbolNameRef symbol,
345345
FileLocalSourceRange range,
346346
clang::FileID fileId,
347347
int32_t allRoles = 0);

‎indexer/ScipExtras.h

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "indexer/Derive.h"
2121
#include "indexer/Enforce.h"
2222
#include "indexer/RAII.h"
23+
#include "indexer/SymbolName.h"
2324

2425
namespace scip {
2526

@@ -114,24 +115,6 @@ class SymbolInformationBuilder final {
114115
void finish(bool deterministic, scip::SymbolInformation &out);
115116
};
116117

117-
class SymbolName {
118-
std::string value;
119-
120-
// The implicitly synthesized copy constructor is important as this is
121-
// used a map key, which are required to be copy-constructible.
122-
public:
123-
SymbolName(std::string &&value) : value(std::move(value)) {
124-
ENFORCE(!this->value.empty());
125-
}
126-
const std::string &asStringRef() const {
127-
return this->value;
128-
}
129-
std::string &asStringRefMut() {
130-
return this->value;
131-
}
132-
DERIVE_HASH_CMP_NEWTYPE(SymbolName, value, CMP_STR)
133-
};
134-
135118
using SymbolToInfoMap = absl::flat_hash_map<
136119
std::string_view,
137120
llvm::PointerUnion<SymbolInformation *, SymbolInformationBuilder *>>;

0 commit comments

Comments
 (0)