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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 10 04:03:13 PDT 2022


kadircet added a comment.

I agree with Nathan to some extent that this doesn't carry as much weight, but if we can get it without introducing much complexity to the user (i.e. a new modifier they need to remember), it's worth having since we don't need too much effort to support it.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:578
     // e.g. std::vector<T>& is dependent but we would want to highlight it
-    if (!T->isLValueReferenceType() ||
-        T.getNonReferenceType().isConstQualified() || T->isDependentType()) {
+    const bool IsRef = T->isLValueReferenceType();
+    const bool IsPtr = T->isPointerType();
----------------
nit: we don't do `const`s for locals (not saying this is a bad practice, it's just not inline with the convention in the rest of the codebase).


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:603
       H.addExtraModifier(*Location,
-                         HighlightingModifier::UsedAsMutableReference);
+                         IsRef ? HighlightingModifier::UsedAsMutableReference
+                               : HighlightingModifier::UsedAsMutablePointer);
----------------
why do you think we should have different modifiers for these? they both indicate the same thing, the parameter might get mutated by the call, and it being a pointer or reference doesn't seem to matter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130015/new/

https://reviews.llvm.org/D130015



More information about the cfe-commits mailing list