[clangd] Support "usedAsMutableReference" in member initializations

[ckandeler]

Summary

That is, mark constructor parameters being used to initialize
non-const reference members.

[nridge]

Thanks, this seems like a useful extension of the usedAsMutableReference modifier

clang-tools-extra/clangd/SemanticHighlighting.cpp

541

Use auto *Member to make clang-tidy happy (const auto * is also fine)

Likewise in a few places below

543

Can we factor out this logic into a helper function highlightMutableReferenceArgument(QualType TargetType, const Expr *Arg)?

Then here we can call highlightMutableReferenceArgument(MemberType, Init->getInit())

553–554

nits from previous patch -- I'm getting clang-tidy diagnostics on this line about:

1. naming (should be CallOp)
2. it seems clang-tidy prefers writing this as auto *CallOp =

Could you fix these as part of this patch?

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

752

Maybe we can add a line where the initializer expression is not a constructor parameter, but e.g. a non-const static data member of another class.

[ckandeler]

Implemented reviewer suggestions.

clang-tools-extra/clangd/SemanticHighlighting.cpp

541

Ewww... But okay.

[ckandeler]

Note that the build failure seems unrelated, as it's due to a failing clang-tidy test.

[nridge]

Thanks!

[ckandeler]

Can you please merge it as well?

[nridge]

Yes, I plan to. I'm just building it locally to verify the tests are passing. (And sometimes, for reasons I don't understand, LLVM's build system triggers a full rebuild even though I've only applied a patch that touches a couple of files...)

[ckandeler]

In D128977#3638198, @nridge wrote:

Yes, I plan to. I'm just building it locally to verify the tests are passing. (And sometimes, for reasons I don't understand, LLVM's build system triggers a full rebuild even though I've only applied a patch that touches a couple of files...)

I see, thanks.
Regarding the scope of rebuilds, it seems to me that the build system pulls in repository meta data, as there's lots of files rebuilt after simply amending a commit with no sources having changed.

clang-tools-extra/clangd/SemanticHighlighting.cpp

if (auto *TD = T->getAsTagDecl())
     return scopeModifier(TD);
   return llvm::None;
 }
 
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
     : public RecursiveASTVisitor<CollectExtraHighlightings> {
+  using Base = RecursiveASTVisitor<CollectExtraHighlightings>;
+
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
     highlightMutableReferenceArguments(E->getConstructor(),
                                        {E->getArgs(), E->getNumArgs()});
 
     return true;
   }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+    if (Init->isMemberInitializer())
+      if (auto *Member = Init->getMember())
+        highlightMutableReferenceArgument(Member->getType(), Init->getInit());
+    return Base::TraverseConstructorInitializer(Init);
+  }
+
   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: consider highlighting parameters of some other overloaded
     // operators as well
     llvm::ArrayRef<const Expr *> Args = {E->getArgs(), E->getNumArgs()};
-    if (const auto callOp = dyn_cast<CXXOperatorCallExpr>(E)) {
-      switch (callOp->getOperator()) {
+    if (auto *CallOp = dyn_cast<CXXOperatorCallExpr>(E)) {
+      switch (CallOp->getOperator()) {
       case OO_Call:
       case OO_Subscript:
         Args = Args.drop_front(); // Drop object parameter
         break;
       default:
         return true;
       }
     }
 
     highlightMutableReferenceArguments(
         dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()), Args);
 
     return true;
   }
 
-  void
-  highlightMutableReferenceArguments(const FunctionDecl *FD,
-                                     llvm::ArrayRef<const Expr *const> Args) {
-    if (!FD)
+  void highlightMutableReferenceArgument(QualType T, const Expr *Arg) {
+    if (!Arg)
       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 The condition T->idDependentType() could be relaxed a bit,
+    if (!T->isLValueReferenceType() ||
+        T.getNonReferenceType().isConstQualified() || T->isDependentType()) {
+      return;
+    }
+
 
-            // 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 (auto *DR = dyn_cast<DeclRefExpr>(Arg))
+    else if (auto *M = dyn_cast<MemberExpr>(Arg))
 
-            if (Location)
-              H.addExtraModifier(*Location,
-                                 HighlightingModifier::UsedAsMutableReference);
-          }
-        }
+
+  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) {
+        highlightMutableReferenceArgument(ProtoType->getParamType(I), Args[I]);
       }
     }
   }
 
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
     if (auto K = kindForType(L.getTypePtr(), H.getResolver())) {
       auto &Tok = H.addToken(L.getBeginLoc(), *K)
                       .addModifier(HighlightingModifier::Deduced);

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

)cpp",
       R"cpp(
         struct $Class_decl[[ClassWithOp]] {
             void operator()(int);
             void operator()(int, int &);
             void operator()(int, int, const int &);
             int &operator[](int &);
             int operator[](int) const;
         };
+        struct $Class_decl[[ClassWithStaticMember]] {
+            static inline int $StaticField_decl_static[[j]] = 0;
+        };
+        struct $Class_decl[[ClassWithRefMembers]] {
+          $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+            : $Field[[i1]]($Parameter[[i]]),
+              $Field_readonly[[i2]]($Parameter[[i]]),
+              $Field[[i3]]($Parameter_usedAsMutableReference[[i]]),
+              $Field_readonly[[i4]]($Class[[ClassWithStaticMember]]::$StaticField_static[[j]]),
+              $Field[[i5]]($Class[[ClassWithStaticMember]]::$StaticField_static_usedAsMutableReference[[j]])
+          {}
+          int $Field_decl[[i1]];
+          const int &$Field_decl_readonly[[i2]];
+          int &$Field_decl[[i3]];
+          const int &$Field_decl_readonly[[i4]];
+          int &$Field_decl[[i5]];
+        };
         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]];