[PATCH] D15486: [RFC] Emit note pointing to a discarded qualifier [-Wincompatible-pointer-types-discards-qualifiers]

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D15486#310572, @adek05 wrote:

> @aaron.ballman I think this could be hard to achieve without an extra note if you have something like:
>
>   cat test2.c
>   int main() {
>           char *c = 'a';
>  
>           char volatile** cc = &c;
>           cc = &c;
>   }
>
>
>
>
>   test2.c:2:15: warning: incompatible integer to pointer conversion initializing 'char *' with an expression of type 'int' [-Wint-conversion]
>           char *c = 'a';
>                 ^   ~~~
>   test2.c:4:25: warning: initializing 'volatile char **' with an expression of type 'char **' discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
>           char volatile** cc = &c;
>                           ^    ~~
>   test2.c:4:9: note: nested pointer type with discarded 'const' qualifier
>           char volatile** cc = &c;
>           ^~~~~~~~~~~~~~~~~~
>   test2.c:5:12: warning: assigning to 'volatile char **' from 'char **' discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
>           cc = &c;
>              ^ ~~
>   3 warnings generated.
>
>
> Sadly, my code doesn't print a note in the second case, which I would have to debug. In case of initialization, I think we can just print one line and omit note. 
>  For assignment which is not an initialization, it might be useful to point to the declaration of a variable which is assigned.
>
> I will try to handle the second case and write tests for this. Let me know if you feel like it is still worth proceeding.


I'm still not convinced this adds value as a note. The diagnostic has the type information for the destination as well as the source. A note pointing where the destination type lives doesn't seem to add any new information, but pointing out which qualifiers are discarded as part of the original diagnostic does add value. So I think that the idea has merit, but I don't think the note is the way to go. Instead, I wonder if it makes sense to just update the existing diagnostic to have further information when it is available.

Also, the note is currently incorrect -- it is claiming that the nested pointer type has a discarded const qualifier, but it has a discarded volatile qualifier. The code needs to figure out which qualifiers are discarded (and those qualifiers may be disjoint in the type specifier). Consider a type like "const char * volatile * const * restrict * const" -- the diagnostic would have to point out which qualifiers are discarded, and it could be that restrict and volatile are discarded but the consts are fine.

Perhaps as a first pass, you could handle the case where only a single qualifier is discarded, and add the qualifier and a SourceRange for that particular qualifier to the existing diagnostic. e.g., `Diag(Loc, diag::warn_incompatible_qualified_id) << FirstType << SecondType << DiscardedQualifier << TypeSourceRange;` Future iterations could then track multiple discarded qualifiers and their source ranges. (Note, I have no idea how easy or hard this might be.)


http://reviews.llvm.org/D15486