diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -69,6 +69,7 @@ Virtual, DependentName, DefaultLibrary, + UsedAsMutableReference, FunctionScope, ClassScope, diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -339,15 +339,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 +354,11 @@ return Tokens.back(); } + void addExtraModifier(SourceLocation Loc, HighlightingModifier Modifier) { + if (auto Range = getRangeForSourceLocation(Loc)) + ExtraModifiers[*Range].push_back(Modifier); + } + std::vector collect(ParsedAST &AST) && { // Initializer lists can give duplicates of tokens, therefore all tokens // must be deduplicated. @@ -377,12 +378,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 +451,23 @@ const HeuristicResolver *getResolver() const { return Resolver; } private: + llvm::Optional 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 Tokens; + std::map> ExtraModifiers; const HeuristicResolver *Resolver; // returned from addToken(InvalidLoc) HighlightingToken InvalidHighlightingToken; @@ -496,6 +520,71 @@ 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(E)) + return true; + + // FIXME ...here it would make sense though. + if (isa(E)) + return true; + + highlightMutableReferenceArguments( + dyn_cast_or_null(E->getCalleeDecl()), + {E->getArgs(), E->getNumArgs()}); + + return true; + } + + void + highlightMutableReferenceArguments(const FunctionDecl *FD, + llvm::ArrayRef Args) { + if (!FD) + return; + + if (auto *ProtoType = FD->getType()->getAs()) { + // 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& is dependent but we would want to highlight it + if (T->isLValueReferenceType() && + !T.getNonReferenceType().isConstQualified() && + !T->isDependentType()) { + if (auto *Arg = Args[I]) { + llvm::Optional 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(Arg)) { + Location = DR->getLocation(); + } else if (auto *M = dyn_cast(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 @@ 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/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test --- 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 --- 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 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -729,6 +729,47 @@ } }; )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 + 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.