[clangd] Also mark output arguments of operator call expressions

[ckandeler]

Summary

There's no reason that arguments to e.g. lambda calls should be treated
differently than those to "real" functions.

[nridge]

Thanks for the patch!

nit: please make the commit message a bit more specific, e.g. "Also apply the 'mutable' semantic token modifier to arguments of overloaded call operators"

clang-tools-extra/clangd/SemanticHighlighting.cpp

539–540

Please update the FIXME to something like:

// FIXME: consider highlighting parameters of some other overloaded operators as well

(There's some discussion here about which other cases would make sense to highlight, and which wouldn't.)

540–547

I think this could be expressed a bit more cleanly with the ArrayRef API:

(Please note also the convention in this codebase to capitalize local variable names.)

llvm::ArrayRef<const Expr *const> Args = {E->getArgs(), E->getNumArgs()};
if (const auto CallOp = ...) {
  if (CallOp->getOperator() != OO_Call)
    return true;
  
  Args = Args.drop_front();  // Drop object parameter
}

highlightMutableReferenceArguments(..., Args);

[ckandeler]

Improvements as per review comments.

[nridge]

Thanks!

I'm going to take the liberty of approving this, as it seems straightforward and unlikely to be contentious in any way.

Please let me know if you need me to commit it.

[ckandeler]

In D128329#3617172, @nridge wrote:

I'm going to take the liberty of approving this, as it seems straightforward and unlikely to be contentious in any way.

Thanks!

Please let me know if you need me to commit it.

Yes, please.

clang-tools-extra/clangd/SemanticHighlighting.cpp

bool VisitCXXConstructExpr(CXXConstructExpr *E) {
     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))
+    // 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)) {
+      if (callOp->getOperator() != OO_Call)
         return true;
+      Args = Args.drop_front(); // Drop object parameter
+    }
 
     highlightMutableReferenceArguments(
-        dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()),
-        {E->getArgs(), E->getNumArgs()});
+        dyn_cast_or_null<FunctionDecl>(E->getCalleeDecl()), Args);
 
     return true;
   }
 
   void
   highlightMutableReferenceArguments(const FunctionDecl *FD,
                                      llvm::ArrayRef<const Expr *const> Args) {
     if (!FD)

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

)cpp",
 
           void $Method_decl[[method]]() {
             (void)$Field_dependentName[[member]];
           }
         };
       )cpp",
       // Modifier for variables passed as non-const references
       R"cpp(
+        struct $Class_decl[[ClassWithOp]] {
+            void operator()(int);
+            void operator()(int, int &);
+            void operator()(int, int, const int &);
+        };
         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]]
                            );
+          [](int){}($LocalVariable[[val]]);
+          [](int&){}($LocalVariable_usedAsMutableReference[[val]]);
+          [](const int&){}($LocalVariable[[val]]);
+          $Class[[ClassWithOp]] $LocalVariable_decl[[c]];
+          $LocalVariable[[c]]($LocalVariable[[val]]);
+          $LocalVariable[[c]](0, $LocalVariable_usedAsMutableReference[[val]]);
+          $LocalVariable[[c]](0, 0, $LocalVariable[[val]]);
         }
         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]]);