[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