[clangd] Add "readonly" token to const member expressions

[t-troebst]

Summary

In semantic highlighting: this adds the readonly token to a in an member expression of the form`a` (implicit this), (...).a or (...)->a if a is const in this context. Previously, only the type of the declaration of a was considered.

[t-troebst]

Changed isConstQualifed() to the custom isConst() (though this isConst() behaves strangely for pointer types, we should fix this separately and keep them synced for now).

[nridge]

Thanks for the patch!

addExtraModifier is a kind of hacky mechanism to attach modifiers in CollectExtraHighlightings to tokens created during findExplicitReferences(); it would be nicer to add the modifier at the time of creating the token, but it doesn't look like we have enough information in ReferenceLoc to do that in this case. We could consider extending ReferenceLoc with more information in the future, but I think this is fine for now.

Do you think it's feasible to make a best-effort attempt to use similar logic for CXXDependentScopeMemberExpr? I don't think it returns a useful getType(), but its getBaseType() should tell us whether the object type is const or not. (It wouldn't handle mutable because we don't have a concrete member decl to work with, but that's a less common case compared to the base type being const or not.)

[t-troebst]

Add best effort attempt to get const-ness for dependent member expressions.

[t-troebst]

I've added some logic to get const-ness for dependent expressions. I think you're right that its probably reasonable to do this since mutable is almost always used in member functions where the type isn't dependent anyways.

[nridge]

clang-tools-extra/clangd/SemanticHighlighting.cpp

1018

getPointeeType() needs a "null" check (check for empty QualType), this can arise if e.g. BaseType is a smart pointer type (E->isArrow() will still be true because the expression syntactically uses ->, but BaseType will not be a PointerType)

Otherwise looks good, thanks!

clang-tools-extra/clangd/SemanticHighlighting.cpp

if (auto *Args = D->getTemplateArgsAsWritten())
       H.addAngleBracketTokens(Args->getLAngleLoc(), Args->getRAngleLoc());
     return true;
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *E) {
     H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
     return true;
   }
+
   bool VisitMemberExpr(MemberExpr *E) {
     H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
+    if (isConst(E->getType()))
+      H.addExtraModifier(E->getMemberLoc(), HighlightingModifier::Readonly);
     return true;
   }
 
   bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
     H.addAngleBracketTokens(L.getLAngleLoc(), L.getRAngleLoc());
     return true;
   }
 
auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::Unknown)
                     .addModifier(HighlightingModifier::DependentName);
     if (llvm::isa<UnresolvedMemberExpr>(E))
       Tok.addModifier(HighlightingModifier::ClassScope);
     // other case is UnresolvedLookupExpr, scope is unknown.
     return true;
   }
 
   bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+    auto &Tok =
         H.addToken(E->getMemberNameInfo().getLoc(), HighlightingKind::Unknown)
             .addModifier(HighlightingModifier::DependentName)
             .addModifier(HighlightingModifier::ClassScope);
+
+    // Best effort attempt to get const-ness for dependent member expressions.
+    // Note: this can yield false-positives if the member is mutable but this
+    // should be rare in dependent contexts.
+    const auto BaseType = E->getBaseType();
+    if ((E->isArrow() && BaseType->getPointeeType().isConstQualified()) ||
+        (!E->isArrow() && BaseType.isConstQualified()))
+      Tok.addModifier(HighlightingModifier::Readonly);
+
     H.addAngleBracketTokens(E->getLAngleLoc(), E->getRAngleLoc());
     return true;
   }
 
   bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
     H.addToken(E->getNameInfo().getLoc(), HighlightingKind::Unknown)
         .addModifier(HighlightingModifier::DependentName)
         .addModifier(HighlightingModifier::ClassScope);

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

const char *TestCases[] = {
         int $Variable_globalScope[[Y]] = $Macro_globalScope[[X]];
       )cpp",
   };
 
   for (const char *Test : TestCases)
     checkHighlightings(Test, {}, ScopeModifierMask);
 }
 
+TEST(SemanticHighlighting, MemberReadonly) {
+  const char *TestCases[] = {
+      R"cpp(
+        struct $Class[[A]] {
+          int $Field[[x]];
+          const int $Field_readonly[[y]];
+          mutable int $Field[[z]];
+        };
+        void $Function[[foo]](){
+          $Class[[A]] $LocalVariable[[a]]{};
+          $LocalVariable[[a]].$Field[[x]];
+          $LocalVariable[[a]].$Field_readonly[[y]];
+          $LocalVariable[[a]].$Field[[z]];
+          const $Class[[A]] $LocalVariable_readonly[[b]]{};
+          $LocalVariable_readonly[[b]].$Field_readonly[[x]];
+          $LocalVariable_readonly[[b]].$Field_readonly[[y]];
+          $LocalVariable_readonly[[b]].$Field[[z]];
+        }
+      )cpp",
+      R"cpp(
+        struct $Class[[A]] {
+          int $Field[[x]];
+          const int $Field_readonly[[y]];
+          mutable int $Field[[z]];
+        };
+        void $Function[[foo]](){
+          $Class[[A]] *$LocalVariable[[a]] = $Operator[[new]] $Class[[A]]{};
+          $LocalVariable[[a]]->$Field[[x]];
+          $LocalVariable[[a]]->$Field_readonly[[y]];
+          $LocalVariable[[a]]->$Field[[z]];
+          const $Class[[A]] *$LocalVariable_readonly[[b]] = $Operator[[new]] $Class[[A]]{};
+          $LocalVariable_readonly[[b]]->$Field_readonly[[x]];
+          $LocalVariable_readonly[[b]]->$Field_readonly[[y]];
+          $LocalVariable_readonly[[b]]->$Field[[z]];
+        }
+      )cpp",
+      R"cpp(
+        struct $Class[[A]] {
+          int $Field[[x]];
+          const int $Field_readonly[[y]];
+          mutable int $Field[[z]];
+          void $Method_readonly[[foo]]() const {
+            $Field_readonly[[x]];
+            $Field_readonly[[y]];
+            $Field[[z]];
+          }
+          void $Method[[bar]]() {
+            $Field[[x]];
+            $Field_readonly[[y]];
+            $Field[[z]];
+          }
+        };
+      )cpp",
+      R"cpp(
+        struct $Class[[A]] {
+          int $Field[[x]];
+          const int $Field_readonly[[y]];
+          mutable int $Field[[z]];
+          void $Method_readonly[[foo]]() const {
+            this->$Field_readonly[[x]];
+            this->$Field_readonly[[y]];
+            this->$Field[[z]];
+          }
+          void $Method[[bar]]() {
+            this->$Field[[x]];
+            this->$Field_readonly[[y]];
+            this->$Field[[z]];
+          }
+        };
+      )cpp",
+      R"cpp(
+        struct $Class[[A]] {
+          void $Method_readonly[[foo]]() const {}
+          void $Method[[foo]]() {}
+          void $Method_readonly[[test]]() const {
+            $Method_readonly[[foo]]();
+          }
+          void $Method[[test]]() {
+            $Method[[foo]]();
+          }
+        };
+      )cpp",
+      R"cpp(
+        template $Bracket[[<]]typename $TemplateParameter[[T]]$Bracket[[>]]
+        void $Function[[foo]]($TemplateParameter[[T]] $Parameter[[x]], const $TemplateParameter[[T]] $Parameter_readonly[[y]]) {
+          $Parameter[[x]].$Unknown[[a]];
+          $Parameter_readonly[[y]].$Unknown_readonly[[b]];
+        }
+      )cpp",
+      R"cpp(
+        template $Bracket[[<]]typename $TemplateParameter[[T]]$Bracket[[>]]
+        void $Function[[foo]]($TemplateParameter[[T]]* $Parameter[[x]], const $TemplateParameter[[T]]* $Parameter_readonly[[y]], $TemplateParameter[[T]]* const $Parameter_readonly[[z]]) {
+          $Parameter[[x]]->$Unknown[[a]];
+          $Parameter_readonly[[y]]->$Unknown_readonly[[b]];
+          $Parameter_readonly[[z]]->$Unknown[[c]];
+        }
+      )cpp",
+  };
+
+  for (const char *Test : TestCases)
+    checkHighlightings(Test, {}, 1 << unsigned(HighlightingModifier::Readonly));
+}
+
 // Ranges are highlighted as variables, unless highlighted as $Function etc.
 std::vector<HighlightingToken> tokens(llvm::StringRef MarkedText) {
   Annotations A(MarkedText);
   std::vector<HighlightingToken> Results;
   for (const Range& R : A.ranges())
     Results.push_back({HighlightingKind::Variable, 0, R});
   for (unsigned I = 0; I < static_cast<unsigned>(HighlightingKind::LastKind); ++I) {
     HighlightingKind Kind = static_cast<HighlightingKind>(I);