On Tue, Jul 15, 2014 at 9:24 AM, David Blaikie <dblai...@gmail.com> wrote:
> >>! In D4479#7, @rtrieu wrote:
> >> Also, would it be better for the diagnostic to point to the "const" in
> the function rather than the "f"?
> >
> > Probably, but the location of the const on a function does not appear to
> be available.
>
> Mr. Smith - any thoughts on whether this should be available? Or is it
> just not feasible due to AST size costs? In any case, if it's not
> available, that's OK for this commit & maybe leave a FIXME in the test and
> in the source suggesting that it could be improved if we get that source
> fidelity in the AST some day.
It *should* be, but it's a historical deficiency in TypeLoc that we do not
model the locations of cv-qualifiers.
> >>! In D4479#6, @rtrieu wrote:
> > ```
> > $ cat c.cc
> > struct foo {
> > int i;
> > void f() const;
> > };
> > void foo::f() cont {
> > i = 3;
> > }
> > $ clang c.cc
> > c.cc:6:5: error: read-only variable is not assignable
> > i = 3;
> > ~ ^
> > c.cc:5:11: note: method 'f' is declared const here
> > void foo::f() const {
> > ~~~~~~~~~~^~~~~~~~~
> > 1 error generated.
> >
> > ```
> > Note will point to function definition.
>
> Great (not sure about the "declared const here" phrasing, though -
> technically true (a definition is a declaration, though most people think
> of the first declaration as "the declaration") and I can't think of much
> better, though)
I think I'd prefer to see:
error: cannot assign to data member 'i' within const member function
note: member function 'foo::f' is declared const here
>
> >
> >
> > ```
> > $ cat d.cc
> > typedef const int X;
> > void test(X x) {
> > x = 5;
> > }
> > $ clang d.cc
> > d.cc:3:5: error: read-only variable is not assignable
> > x = 5;
> > ~ ^
> > d.cc:2:13: note: variable 'x' is declared here with type 'X' (aka 'const
> int') which is const
> > void test(X x) {
> > ~~^
> > 1 error generated.
> >
> > ```
> > No problems with typedefs.
> >
> > I think the note should explicitly point out the const qualifier,
> otherwise the user will see "note: variable 'x' has type 'y'" which isn't
> very helpful.
>
> Depends if the 'y' has the word 'const' in it, in which case it's fairly
> helpful. Though that's why I was suggesting possibly repurposing the type
> diffing code to highlight the "const"-ness.
I'd prefer:
error: cannot assign to variable 'x' with const-qualified type 'y'
note: declared here
> > Perhaps "note: variable "x" is declared here with type 'const int';
> const qualifier prevents assignment" would be better?
>
> That sounds pretty good to me. Would have to see it in a few more examples
> (like the const this pointer case, etc) to see how the phrasing generalizes.
>
Generally, I think we should put enough information in the error that the
user can get a general sense of what's wrong before they read the notes, so
I'd like to see the headline information moved from the notes to the errors
in this case.
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
