tom-anders created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang-tools-extra.
See https://github.com/clangd/clangd/issues/839
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108320
Files:
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
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -729,6 +729,28 @@
}
};
)cpp",
+ // Modifier for variables passed as non-const references
+ R"cpp(
+ void $Function_decl[[fun]](int $Parameter_decl[[value]], const int& $Parameter_decl_readonly[[constRef]],
+ int& $Parameter_decl[[ref]], int* $Parameter_decl[[ptr]],
+ int $Parameter_decl[[defaultParameter]] = 3);
+ struct $Class_decl[[S]] {
+ $Class_decl[[S]](int $Parameter_decl[[value]], const int& $Parameter_decl_readonly[[constRef]],
+ int& $Parameter_decl[[ref]], int* $Parameter_decl[[ptr]],
+ int $Parameter_decl[[defaultParameter]] = 3);
+ int $Field_decl[[field]];
+ };
+ void $Function_decl[[bar]]() {
+ int $LocalVariable_decl[[foo]];
+ $Function[[fun]]($LocalVariable[[foo]], $LocalVariable[[foo]],
+ $LocalVariable_passedByNonConstRef[[foo]], &$LocalVariable[[foo]]);
+
+ $Class[[S]] $LocalVariable_decl[[s]]($LocalVariable[[foo]], $LocalVariable[[foo]],
+ $LocalVariable_passedByNonConstRef[[foo]], &$LocalVariable[[foo]]);
+ $Function[[fun]]($LocalVariable[[s]].$Field[[field]], $LocalVariable[[s]].$Field[[field]],
+ $LocalVariable[[s]].$Field_passedByNonConstRef[[field]], &$LocalVariable[[s]].$Field[[field]]);
+ }
+ )cpp",
};
for (const auto &TestCase : TestCases)
// Mask off scope modifiers to keep the tests manageable.
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===================================================================
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ 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: }
Index: clang-tools-extra/clangd/test/initialize-params.test
===================================================================
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -91,6 +91,7 @@
# CHECK-NEXT: "virtual",
# CHECK-NEXT: "dependentName",
# CHECK-NEXT: "defaultLibrary",
+# CHECK-NEXT: "passedByNonConstRef",
# CHECK-NEXT: "functionScope",
# CHECK-NEXT: "classScope",
# CHECK-NEXT: "fileScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -70,6 +70,8 @@
DependentName,
DefaultLibrary,
+ PassedByNonConstRef,
+
FunctionScope,
ClassScope,
FileScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -290,10 +290,12 @@
}
unsigned evaluateHighlightPriority(const HighlightingToken &Tok) {
- enum HighlightPriority { Dependent = 0, Resolved = 1 };
+ enum HighlightPriority { Dependent = 0, ResolvedButUnknown = 1, Resolved = 2 };
+
return (Tok.Modifiers & (1 << uint32_t(HighlightingModifier::DependentName)))
? Dependent
- : Resolved;
+ : (Tok.Kind == HighlightingKind::Unknown ? ResolvedButUnknown
+ : Resolved);
}
// Sometimes we get multiple tokens at the same location:
@@ -309,6 +311,8 @@
//
// - token kinds that come with "dependent-name" modifiers are less reliable
// (these tend to be vague, like Type or Unknown)
+// - Resolved tokens (i.e. without the "dependent-name" modifier) with kind
+// "Unknown" are less reliable than resolved tokens with other kinds
// - if we have multiple equally reliable kinds, drop token rather than guess
// - take the union of modifiers from all tokens
//
@@ -494,7 +498,65 @@
class CollectExtraHighlightings
: public RecursiveASTVisitor<CollectExtraHighlightings> {
public:
- CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
+ CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {
+ }
+
+ bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+ highlightPassedByNonConstReference(
+ E->getConstructor(), [&](size_t I) -> Expr * {
+ return I < E->getNumArgs() ? E->getArg(I) : nullptr;
+ });
+
+ return true;
+ }
+
+ bool VisitCallExpr(CallExpr *E) {
+ // Highlighting parameters passed by non-const reference does not really
+ // make sence for these
+ if (isa<CXXOperatorCallExpr>(E) || isa<UserDefinedLiteral>(E))
+ return true;
+
+ highlightPassedByNonConstReference(
+ dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()),
+ [&](size_t I) -> Expr * {
+ return I < E->getNumArgs() ? E->getArg(I) : nullptr;
+ });
+
+ return true;
+ }
+
+ void highlightPassedByNonConstReference(
+ const FunctionDecl *FD,
+ llvm::function_ref<Expr *(size_t)> GetArgExprAtIndex) {
+ if (!FD)
+ return;
+
+ // Iterate over the declarations of the function parameters.
+ // If any of them are non-const reference paramters, add it as a
+ // highlighting modifier to the corresponding expression
+ for (size_t I = 0; I < FD->getNumParams(); ++I) {
+ if (const auto *Param = FD->getParamDecl(I)) {
+ auto T = Param->getType();
+
+ // Is this parameter passed by non-const reference?
+ if (T->isLValueReferenceType() && !isConst(T)) {
+ if (auto *Arg = GetArgExprAtIndex(I)) {
+ llvm::Optional<SourceLocation> Location;
+
+ if (isa<DeclRefExpr>(Arg)) {
+ Location = Arg->getBeginLoc();
+ } else if (auto *M = dyn_cast<MemberExpr>(Arg)) {
+ Location = M->getMemberLoc();
+ }
+
+ if (Location)
+ H.addToken(*Location, HighlightingKind::Unknown)
+ .addModifier(HighlightingModifier::PassedByNonConstRef);
+ }
+ }
+ }
+ }
+ }
bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
if (auto K = kindForType(L.getTypePtr(), H.getResolver())) {
@@ -912,6 +974,8 @@
return "dependentName"; // nonstandard
case HighlightingModifier::DefaultLibrary:
return "defaultLibrary";
+ case HighlightingModifier::PassedByNonConstRef:
+ return "passedByNonConstRef"; // nonstandard
case HighlightingModifier::FunctionScope:
return "functionScope"; // nonstandard
case HighlightingModifier::ClassScope:
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits