[PATCH] D37254: [Sema] Disallow assigning record lvalues with nested const-qualified fields.

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5888
 
 def note_typecheck_assign_const : Note<
   "%select{"
----------------
Shouldn't there be one more entry here as well?
(see comment about updating both err_typecheck_assign_const and note_typecheck_assign_const when adding things to the enum at line 10181).

If I understand it correctly you never print notes for the new NestedConstMember, so there isn't really a need for a fifth entry in the select today.
But if someone adds new enum values later (putting them after NestedContMember) then it might be nice if the size of this select matches with the old size of the enum.



================
Comment at: lib/Sema/SemaExpr.cpp:10329
+      S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
+          << ConstMember << false /*non-static*/ << Field
+          << Field->getType() << Field->getSourceRange();
----------------
If adding a new entry to note_typecheck_assign_const (identical to the ConstMember entry) then you can use NestedConstMember here. It would make it possible to change the string for NestedConstMember notes without also changing all ConstMember notes.

Now it kind of looks like an error that the error diagnostic is for a NestedConstMember, but the note is about a ConstMember. That might confuse someone, since it looks like a typo unless you know what is going on here.


https://reviews.llvm.org/D37254