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

[Richard Smith]

+  "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.


On Fri, Aug 1, 2014 at 5:43 PM, Richard Trieu <rtrieu at google.com> wrote:

> Improve the wording of the error message.  Make the error more informative
> by taking the information from the notes and moving it to the error message.
>
> http://reviews.llvm.org/D4479
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/Sema/SemaExpr.cpp
>   test/CXX/temp/temp.decls/temp.mem/p5.cpp
>   test/Sema/block-misc.c
>   test/SemaCXX/anonymous-union.cpp
>   test/SemaCXX/captured-statements.cpp
>   test/SemaCXX/cxx0x-constexpr-const.cpp
>   test/SemaCXX/err_typecheck_assign_const.cpp
>   test/SemaCXX/function-type-qual.cpp
>   test/SemaObjC/arc.m
>   test/SemaTemplate/dependent-type-identity.cpp
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140812/0c13b647/attachment.html>