[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