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

David Blaikie dblaikie at gmail.com
Tue Jul 15 09:24:54 PDT 2014


>>! 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.

>>! 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)

> 
> 
> 
> ```
> $ 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.

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

Open to other people's ideas too, hopefully there's some other opinions - mine are by no means "golden".

http://reviews.llvm.org/D4479






More information about the cfe-commits mailing list