[PATCH] D128329: [clangd] Also mark output arguments of operator call expressions

Nathan Ridge via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 26 21:55:31 PDT 2022


nridge added a comment.

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"



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:540
 
     // FIXME ...here it would make sense though.
+    Expr **args = nullptr;
----------------
Please update the FIXME to something like:

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

(There's some discussion [here](https://reviews.llvm.org/D108320?id=367302#inline-1031934) about which other cases would make sense to highlight, and which wouldn't.)


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:541
     // FIXME ...here it would make sense though.
-    if (isa<CXXOperatorCallExpr>(E))
-      return true;
+    Expr **args = nullptr;
+    unsigned numArgs = 0;
----------------
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);
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128329/new/

https://reviews.llvm.org/D128329



More information about the cfe-commits mailing list