I like this a lot! How does it fare against more complicated macros, though?
#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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
