[cfe-commits] [PATCH] more useful 'duplicate case' diagnostics

[Douglas Gregor]

On May 15, 2012, at 4:50 PM, Terry Long wrote:

> Attached is a new version of the patch for review.
> I have removed all of the textual parsing and replaced it with code that finds a DeclRefExpr if it exists.
> Unfortunately, this will catch a lot less cases, but at least it will catch some common ones and display them nicely.

I like this patch a lot, and I think it's a good trade-off between complexity and functionality. Thank you! Committed as r156904.

	- Doug

> -Terry
> 
> <PR9243v2.patch>
> 
> Am 15.05.2012 um 18:41 schrieb Douglas Gregor:
> 
>> 
>> On May 15, 2012, at 3:28 PM, Terry Long <macmantrl at me.com> wrote:
>> 
>>> 
>>> Am 15.05.2012 um 16:09 schrieb Douglas Gregor
>>>> 
>>>> 
>>>> I'm a bit uncomfortable with the textual approach here. Why not analyze the expressions to check whether they are references to named declarations, and use those names in the diagnostic? It won't catch some macro cases, but it's far simpler.
>>> 
>>> I was also uncomfortable with the textual approach, and in fact I first tried seeing if the case expression was a DeclRefExpr that refers to a valid NamedDecl. The problem I found with that approach was that it was not catching very many situations. Doing it that way only works for enums as far as I can tell. Macros, const ints, case overflows, casting, and more complicated expressions (like "case 2+2") are not DeclRefExprs, and I don't see any obvious way to grab the necessary diagnostic info without using a textual approach.
>> 
>> 
>> You can look through implicit conversions (e.g., promotions) and parentheses to find the underlying expression. We'll get a DeclRefExpr for enumerators and variables that are constant ("const int x = 17;"), which are presumably the common cases. We won't get macros, of course, but we might not want to: it's probably more useful to show the value we get when the macro is expanded, and let the macro-substitution backtrace show how we got there.
>> 
>> 	- Doug
>