[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