[PATCH] Add more information when displaying a "read-only variable is not assignable" error

Richard Smith richard at metafoo.co.uk
Tue Aug 12 12:48:34 PDT 2014


+  "cannot assign to const data member %1 with const qualified type %2|"

Replace the "const data member" with "data member" here; we don't need to
point out the constness twice. (Likewise below)

+  "cannot assign to data member within const member function %1|"

"data member" is not the right term here. Use either "non-static data
member" or "field" (we tend to use the latter in diagnostics that might
appear in C, and the former in exclusively C++ diagnostics).

+  "cannot assign to const data member %1 with const qualified type %2 in a
"
+  "const member function %3}0">;

I don't think this last variant is useful: I think we should either point
out that the field's type is const, or that we're in a const member
function, not both. The way the diagnostic is phrased now might lead to
people thinking that removing *either* const would solve the problem.
Diagnosing the constness of the field seems better to me, since seems in
some sense more fundamental.

+def note_function_returns_const_ref : Note<
+  "function %0 which returns const qualified type %1 declared here">;
+def note_const_variable : Note<
+  "variable %0 declared const here">;
+def note_const_field : Note<
+  "field %0 declared const here">;
+def note_const_method : Note<
+  "member function %q0 is declared const here">;

Could you use a single diagnostic with a %select here?

+enum {
+  ConstUnknown,
+  ConstFunction,
+  ConstVariable,
+  ConstField,
+  ConstMethod,
+  ConstFieldAndMethod
+};

Move this inside the function.

+  PartialDiagnostic PD = S.PDiag(diag::err_typecheck_assign_const)
+                         << E->getSourceRange();

Seems like weird indentation for the <<.


+  // Handle class fields and methods.
+  if (const MemberExpr *Member = dyn_cast<MemberExpr>(E)) {
+    const Expr *Base = E;
+    while (Member) {
+      ValueDecl *VD = Member->getMemberDecl();
+      // Class field is const.
+      if (VD->getType().isConstQualified()) {
+        ME = Member;
+        break;
+      }
+      Base = Member->getBase();
+      Member = dyn_cast<MemberExpr>(Base);
+    }

This loop should go before all the other checks, so that we can diagnose

  const X x;
  x.y.z = 3;

  const X &f();
  f().y.z = 3;

properly.

http://reviews.llvm.org/D4479






More information about the cfe-commits mailing list