nridge added a comment.
In D130015#3702004 <https://reviews.llvm.org/D130015#3702004>, @ckandeler wrote:
> IMO the relevant point is the (non-)const-ness of the corresponding
> parameter, i.e. can the passed object get mutated or not. The fact that
> there's an ampersand at the call site (which is not even the case if the
> variable is itself a pointer) does not tell me that.
Taking another look at this, I find this use case convincing and I'm fine with
taking this patch.
I do prefer the separate modifier for customizability, as I think the
desirability of highlighting this can vary based on the coding convention. For
example, one project I work with is slightly older and a lot of objects are
passed around by non-const pointer, but actual "`out` parameters" almost always
take the form of `&var` at the call site **or** use a reference; in such a
case, the pointer modifier would be fairly noisy and it's nice to be able to
disable it while keeping the reference modifier on.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:587
// FIXME Add "unwrapping" for ArraySubscriptExpr and UnaryOperator,
// e.g. highlight `a` in `a[i]`
----------------
nit: you can remove the "and UnaryOperator" part of the FIXME, since you're
addressing it
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:592
+ Arg = IC->getSubExprAsWritten();
+ if (auto *UO = dyn_cast<UnaryOperator>(Arg)) {
+ if (UO->getOpcode() == UO_AddrOf)
----------------
Could you add a test case that exercises the `UnaryOperator` case?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130015/new/
https://reviews.llvm.org/D130015
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
