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

Richard Trieu rtrieu at google.com
Wed Aug 13 14:52:10 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).
removed const;

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

  +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 <<.
Added two more spances.

  + 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;

Added test cases and makes sure test runs proberl


More information about the cfe-commits mailing list