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

[Terry Long]

> I like this a lot! How does it fare against more complicated macros, though?

As is it doesn't display very helpful diagnostics for complicated macros like these, and even crashes on some.
A good reason not to try to do textual parsing.


> 
> #define CASE1 case 1
> #define CASE(x) case x
> #define MULTI_CASE(x,y) case x: case y:
> 
> switch (x) {
> CASE1: break;
> CASE(1U): break;
> MULTI_CASE(2,2): break;
> case 2: break;
> }
> 
> As for multi-line case statements, I'd be inclined to simply fall back to the dumb version of the warning, rather than trying to format it nicely in the output. We already have a note showing where the relevant code is. And I wouldn't be worried about casts -- if they're there, the user probably thought they were making a difference. (I'd actually be more worried about things like parens, but I'd err on the side of including them anyway.)
> 
> Jordy
> 
> 
> On May 15, 2012, at 12:05, Terry Long wrote:
> 
>> Hi all,
>> 
>> This patch mostly inspired by http://llvm.org/bugs/show_bug.cgi?id=9243 -- poor "duplicate case" diagnostic with enums
>> 
>> Previously the diagnostic for duplicate case values in switch statements was always something like "duplicate case value '5'" 
>> 
>> What my patch changes:
>> When the duplicate case is exactly the same in textual representation (in source code), the diagnostic will use that exact text in the diagnostic.
>> e.g. using MyEnum twice gives the diagnostic "duplicate case value 'MyEnum'"
>> 
>> If the textual representations differ, the new diagnostic explicitly states they are equal to the same value
>> e.g. using MyEnum (equal to 5) and 5, the diagnostic displays "duplicate case value: 'MyEnum' and '5' both equal '5'"
>> 
>> The patch does the same for any valid expression in a case statement (including macros, const ints, char literals, etc.).
>> 
>> Concerns with my patch:
>> 1. Multiline case statements, although unusual, will be printed out in the diagnostic exactly as they appear in the source. I'm not sure if this is a real issue. Possible solution would be checking if it spans multiple lines. If so, the diagnostic could be "duplicate case value: both expressions equal 'x'".
>> e.g.
>> case 1 +
>> 2 +
>> 3:
>> 
>> 2. If a case statement includes a cast, the cast will also be included in the diagnostic. Sometimes this inclusion is helpful, for example in situations where the cast will cause an overflow (such as "(char)256" and "0"). Other times it is irrelevant, such as "(int)7" and "7", where a simple diagnostic of "duplicate case value '7'" makes more sense. I'm also not sure if this is an issue.
>> 
>> 
>> Thanks!
>> 
>> Terry Long
>> 
>> <PR9243.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>