tom-anders updated this revision to Diff 368177.
tom-anders marked 3 inline comments as done.
tom-anders added a comment.
- Apply suggested renaming and fix nits
- Use a map for extra modifiers instead of abusing conflict resolution
- Use ArrayRef instead of callback
- Add FIXME for handling CXXOperatorCallExpr
- Use getLocation() instead of getBeginLoc()
- Add FIXME for unwrapping operators
- Iterate over FunctionProtoType instead of FunctionDecl
- Adjust highlighting condition and revise test cases
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108320/new/
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,63 @@
}
};
)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",
+ /*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_usedAsMutableReference[[foo]], &$LocalVariable[[foo]]);
+
+ $Class[[S]] $LocalVariable_decl[[s]]($LocalVariable[[foo]], $LocalVariable[[foo]],
+ $LocalVariable_usedAsMutableReference[[foo]], &$LocalVariable[[foo]]);
+ $Function[[fun]]($LocalVariable[[s]].$Field[[field]], $LocalVariable[[s]].$Field[[field]],
+ $LocalVariable[[s]].$Field_usedAsMutableReference[[field]], &$LocalVariable[[s]].$Field[[field]]);
+ }*/
};
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: "usedAsMutableReference",
# 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
@@ -69,6 +69,7 @@
Virtual,
DependentName,
DefaultLibrary,
+ UsedAsMutableReference,
FunctionScope,
ClassScope,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -291,6 +291,7 @@
unsigned evaluateHighlightPriority(const HighlightingToken &Tok) {
enum HighlightPriority { Dependent = 0, Resolved = 1 };
+
return (Tok.Modifiers & (1 << uint32_t(HighlightingModifier::DependentName)))
? Dependent
: Resolved;
@@ -339,15 +340,11 @@
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 +355,11 @@
return Tokens.back();
}
+ void addExtraModifier(SourceLocation Loc, HighlightingModifier Modifier) {
+ if (auto Range = getRangeForSourceLocation(Loc))
+ ExtraModifiers[*Range].insert(Modifier);
+ }
+
std::vector<HighlightingToken> collect(ParsedAST &AST) && {
// Initializer lists can give duplicates of tokens, therefore all tokens
// must be deduplicated.
@@ -377,12 +379,22 @@
// 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 different 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 +452,22 @@
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, std::set<HighlightingModifier>> ExtraModifiers;
const HeuristicResolver *Resolver;
// returned from addToken(InvalidLoc)
HighlightingToken InvalidHighlightingToken;
@@ -496,6 +520,63 @@
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 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 < std::min(size_t(ProtoType->getNumParams()), Args.size()); ++I) {
+ auto T = ProtoType->getParamType(I);
+
+ // Is this parameter passed by non-const reference?
+ 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]`
+ 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 +993,8 @@
return "dependentName"; // nonstandard
case HighlightingModifier::DefaultLibrary:
return "defaultLibrary";
+ case HighlightingModifier::UsedAsMutableReference:
+ return "usedAsMutableReference"; // nonstandard
case HighlightingModifier::FunctionScope:
return "functionScope"; // nonstandard
case HighlightingModifier::ClassScope:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
