[PATCH] D130015: [clangd] Add "usedAsMutablePointer" highlighting modifier

[Nathan Ridge via Phabricator via cfe-commits]

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