[clang-tools-extra] 5a85f9b - Add semantic token modifier for non-const reference parameter
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 12 21:50:39 PDT 2021
Author: Tom Praschan
Date: 2021-09-13T00:51:09-04:00
New Revision: 5a85f9b1d48c4367bf697adc0f62ed5c9378f0f3
URL: https://github.com/llvm/llvm-project/commit/5a85f9b1d48c4367bf697adc0f62ed5c9378f0f3
DIFF: https://github.com/llvm/llvm-project/commit/5a85f9b1d48c4367bf697adc0f62ed5c9378f0f3.diff
LOG: Add semantic token modifier for non-const reference parameter
See https://github.com/clangd/clangd/issues/839
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D108320
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 67315f5b99e06..d55bd9e459d53 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -339,15 +339,11 @@ class HighlightingsBuilder {
LangOpts(AST.getLangOpts()) {}
HighlightingToken &addToken(SourceLocation Loc, HighlightingKind Kind) {
- Loc = getHighlightableSpellingToken(Loc, SourceMgr);
- if (Loc.isInvalid())
+ auto Range = getRangeForSourceLocation(Loc);
+ if (!Range)
return InvalidHighlightingToken;
- const auto *Tok = TB.spelledTokenAt(Loc);
- assert(Tok);
- return addToken(
- halfOpenToRange(SourceMgr,
- Tok->range(SourceMgr).toCharRange(SourceMgr)),
- Kind);
+
+ return addToken(*Range, Kind);
}
HighlightingToken &addToken(Range R, HighlightingKind Kind) {
@@ -358,6 +354,11 @@ class HighlightingsBuilder {
return Tokens.back();
}
+ void addExtraModifier(SourceLocation Loc, HighlightingModifier Modifier) {
+ if (auto Range = getRangeForSourceLocation(Loc))
+ ExtraModifiers[*Range].push_back(Modifier);
+ }
+
std::vector<HighlightingToken> collect(ParsedAST &AST) && {
// Initializer lists can give duplicates of tokens, therefore all tokens
// must be deduplicated.
@@ -377,12 +378,22 @@ class HighlightingsBuilder {
// this predicate would never fire.
return T.R == TokRef.front().R;
});
- if (auto Resolved = resolveConflict(Conflicting))
+ if (auto Resolved = resolveConflict(Conflicting)) {
+ // Apply extra collected highlighting modifiers
+ auto Modifiers = ExtraModifiers.find(Resolved->R);
+ if (Modifiers != ExtraModifiers.end()) {
+ for (HighlightingModifier Mod : Modifiers->second) {
+ Resolved->addModifier(Mod);
+ }
+ }
+
NonConflicting.push_back(*Resolved);
+ }
// TokRef[Conflicting.size()] is the next token with a
diff erent range (or
// the end of the Tokens).
TokRef = TokRef.drop_front(Conflicting.size());
}
+
const auto &SM = AST.getSourceManager();
StringRef MainCode = SM.getBufferOrFake(SM.getMainFileID()).getBuffer();
@@ -440,10 +451,23 @@ class HighlightingsBuilder {
const HeuristicResolver *getResolver() const { return Resolver; }
private:
+ llvm::Optional<Range> getRangeForSourceLocation(SourceLocation Loc) {
+ Loc = getHighlightableSpellingToken(Loc, SourceMgr);
+ if (Loc.isInvalid())
+ return llvm::None;
+
+ const auto *Tok = TB.spelledTokenAt(Loc);
+ assert(Tok);
+
+ return halfOpenToRange(SourceMgr,
+ Tok->range(SourceMgr).toCharRange(SourceMgr));
+ }
+
const syntax::TokenBuffer &TB;
const SourceManager &SourceMgr;
const LangOptions &LangOpts;
std::vector<HighlightingToken> Tokens;
+ std::map<Range, llvm::SmallVector<HighlightingModifier, 1>> ExtraModifiers;
const HeuristicResolver *Resolver;
// returned from addToken(InvalidLoc)
HighlightingToken InvalidHighlightingToken;
@@ -496,6 +520,71 @@ class CollectExtraHighlightings
public:
CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
+ bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+ highlightMutableReferenceArguments(E->getConstructor(),
+ {E->getArgs(), E->getNumArgs()});
+
+ return true;
+ }
+
+ bool VisitCallExpr(CallExpr *E) {
+ // Highlighting parameters passed by non-const reference does not really
+ // make sense for literals...
+ if (isa<UserDefinedLiteral>(E))
+ return true;
+
+ // FIXME ...here it would make sense though.
+ if (isa<CXXOperatorCallExpr>(E))
+ return true;
+
+ highlightMutableReferenceArguments(
+ dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()),
+ {E->getArgs(), E->getNumArgs()});
+
+ return true;
+ }
+
+ void
+ highlightMutableReferenceArguments(const FunctionDecl *FD,
+ llvm::ArrayRef<const Expr *const> Args) {
+ if (!FD)
+ return;
+
+ if (auto *ProtoType = FD->getType()->getAs<FunctionProtoType>()) {
+ // Iterate over the types of the function parameters.
+ // If any of them are non-const reference paramteres, add it as a
+ // highlighting modifier to the corresponding expression
+ for (size_t I = 0;
+ I < std::min(size_t(ProtoType->getNumParams()), Args.size()); ++I) {
+ auto T = ProtoType->getParamType(I);
+
+ // Is this parameter passed by non-const reference?
+ // FIXME The condition !T->idDependentType() could be relaxed a bit,
+ // e.g. std::vector<T>& is dependent but we would want to highlight it
+ if (T->isLValueReferenceType() &&
+ !T.getNonReferenceType().isConstQualified() &&
+ !T->isDependentType()) {
+ if (auto *Arg = Args[I]) {
+ llvm::Optional<SourceLocation> Location;
+
+ // FIXME Add "unwrapping" for ArraySubscriptExpr and UnaryOperator,
+ // e.g. highlight `a` in `a[i]`
+ // FIXME Handle dependent expression types
+ if (auto *DR = dyn_cast<DeclRefExpr>(Arg)) {
+ Location = DR->getLocation();
+ } else if (auto *M = dyn_cast<MemberExpr>(Arg)) {
+ Location = M->getMemberLoc();
+ }
+
+ if (Location)
+ H.addExtraModifier(*Location,
+ HighlightingModifier::UsedAsMutableReference);
+ }
+ }
+ }
+ }
+ }
+
bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
if (auto K = kindForType(L.getTypePtr(), H.getResolver())) {
auto &Tok = H.addToken(L.getBeginLoc(), *K)
@@ -912,6 +1001,8 @@ llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) {
return "dependentName"; // nonstandard
case HighlightingModifier::DefaultLibrary:
return "defaultLibrary";
+ case HighlightingModifier::UsedAsMutableReference:
+ return "usedAsMutableReference"; // nonstandard
case HighlightingModifier::FunctionScope:
return "functionScope"; // nonstandard
case HighlightingModifier::ClassScope:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h
index 10bcb47ef45b5..b44aa505b4ba3 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -69,6 +69,7 @@ enum class HighlightingModifier {
Virtual,
DependentName,
DefaultLibrary,
+ UsedAsMutableReference,
FunctionScope,
ClassScope,
diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index 891ef2c3ad9a5..d356c2e0f7652 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -91,6 +91,7 @@
# CHECK-NEXT: "virtual",
# CHECK-NEXT: "dependentName",
# CHECK-NEXT: "defaultLibrary",
+# CHECK-NEXT: "usedAsMutableReference",
# CHECK-NEXT: "functionScope",
# CHECK-NEXT: "classScope",
# CHECK-NEXT: "fileScope",
diff --git a/clang-tools-extra/clangd/test/semantic-tokens.test b/clang-tools-extra/clangd/test/semantic-tokens.test
index 6d3e324860edd..a29993bc9ba28 100644
--- a/clang-tools-extra/clangd/test/semantic-tokens.test
+++ b/clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 4097
+# CHECK-NEXT: 8193
# CHECK-NEXT: ],
# CHECK-NEXT: "resultId": "1"
# CHECK-NEXT: }
@@ -49,7 +49,7 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 4097
+# CHECK-NEXT: 8193
# CHECK-NEXT: ],
# Inserted at position 1
# CHECK-NEXT: "deleteCount": 0,
@@ -72,12 +72,12 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 4097,
+# CHECK-NEXT: 8193,
# CHECK-NEXT: 1,
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 4097
+# CHECK-NEXT: 8193
# CHECK-NEXT: ],
# CHECK-NEXT: "resultId": "3"
# CHECK-NEXT: }
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 403f506824176..646c6ec184652 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -729,6 +729,47 @@ sizeof...($TemplateParameter[[Elements]]);
}
};
)cpp",
+ // Modifier for variables passed as non-const references
+ R"cpp(
+ void $Function_decl[[fun]](int, const int,
+ int*, const int*,
+ int&, const int&,
+ int*&, const int*&, const int* const &,
+ int**, int**&, int** const &,
+ int = 123) {
+ int $LocalVariable_decl[[val]];
+ int* $LocalVariable_decl[[ptr]];
+ const int* $LocalVariable_decl_readonly[[constPtr]];
+ int** $LocalVariable_decl[[array]];
+ $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]],
+ $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]],
+ $LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]],
+
+ $LocalVariable_usedAsMutableReference[[ptr]],
+ $LocalVariable_readonly_usedAsMutableReference[[constPtr]],
+ $LocalVariable_readonly[[constPtr]],
+
+ $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]],
+ $LocalVariable[[array]]
+ );
+ }
+ struct $Class_decl[[S]] {
+ $Class_decl[[S]](int&) {
+ $Class[[S]] $LocalVariable_decl[[s1]]($Field_usedAsMutableReference[[field]]);
+ $Class[[S]] $LocalVariable_decl[[s2]]($LocalVariable[[s1]].$Field_usedAsMutableReference[[field]]);
+
+ $Class[[S]] $LocalVariable_decl[[s3]]($StaticField_static_usedAsMutableReference[[staticField]]);
+ $Class[[S]] $LocalVariable_decl[[s4]]($Class[[S]]::$StaticField_static_usedAsMutableReference[[staticField]]);
+ }
+ int $Field_decl[[field]];
+ static int $StaticField_decl_static[[staticField]];
+ };
+ template <typename $TemplateParameter_decl[[X]]>
+ void $Function_decl[[foo]]($TemplateParameter[[X]]& $Parameter_decl[[x]]) {
+ // We do not support dependent types, so this one should *not* get the modifier.
+ $Function[[foo]]($Parameter[[x]]);
+ }
+ )cpp",
};
for (const auto &TestCase : TestCases)
// Mask off scope modifiers to keep the tests manageable.
More information about the cfe-commits
mailing list