[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())

563

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())
+      return Base::TraverseConstructorInitializer(Init);
+    if (const auto Member = Init->getMember()) {
+      const auto MemberType = Member->getType();
+      if (MemberType->isLValueReferenceType() &&
+          !MemberType->getPointeeType().isConstQualified()) {
+        if (const auto Param = dyn_cast<DeclRefExpr>(Init->getInit())) {
+          H.addExtraModifier(Param->getLocation(),
+                             HighlightingModifier::UsedAsMutableReference);
+        }
+      }
+    }
+    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()) {
       case OO_Call:
       case OO_Subscript:
         Args = Args.drop_front(); // Drop object parameter
         break;
       default:
         return true;
       }

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[[ClassWithRefMembers]] {
+          $Class_decl[[ClassWithRefMembers]](int $Parameter_decl[[i]])
+            : $Field[[i1]]($Parameter[[i]]),
+              $Field_readonly[[i2]]($Parameter[[i]]),
+              $Field[[i3]]($Parameter_usedAsMutableReference[[i]]) {}
+          int $Field_decl[[i1]];
+          const int &$Field_decl_readonly[[i2]];
+          int &$Field_decl[[i3]];
+        };
         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]];