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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
