felipeprov / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
0 stars 0 forks source link

-Wcovered-switch-default warnings #117

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Steps to reproduce:
1. Build IWYU in-tree with clang.

Actual result:
There are a few warnings like
`warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]`

Expected result:
No warnings.

Notes:
Attached patch fixes the warning. Please check it doesn't break anything else. 
Also see [1] for more -Wcovered-switch-default details.

[1] 
http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-cove
red-switches-over-enumerations

Original issue reported on code.google.com by vsap...@gmail.com on 30 Dec 2013 at 3:53

Attachments:

GoogleCodeExporter commented 8 years ago
Both MSVC and GCC seem happy with this.

Have you considered using llvm_unreachable? I think CHECK_ has the same effect, 
but it could be that the unconditional llvm_unreachable lets you remove the 
bogus return values. See uses of llvm_unreachable in the Clang codebase.

This looks good to me as-is, but I think it'd be even nicer with 
llvm_unreachable.

Original comment by kim.gras...@gmail.com on 30 Dec 2013 at 4:19

GoogleCodeExporter commented 8 years ago
-Wcovered-switch-default and llvm_unreachable occur sometimes together, but are 
independent. So I've committed warning fix to avoid mixing everything together 
in a single commit. As for llvm_unreachable see attached patch.

I've considered using llvm_unreachable as is, but CHECK_ assertions aren't 
disabled in Release build and I don't want CHECK_ and llvm_unreachable 
assertions to behave differently in Debug and Release. Also CHECK_ returns a 
stream but llvm_unreachable doesn't. That's why I've introduced 
CHECK_UNREACHABLE_ making ~FatalMessageEmitter() more like 
llvm::llvm_unreachable_internal() (from "llvm/Support/ErrorHandling.h"). What 
do you think, Kim?

Original comment by vsap...@gmail.com on 30 Dec 2013 at 8:24

Attachments:

GoogleCodeExporter commented 8 years ago
Looks good! I tested on MSVC; built and ran fine.

Original comment by kim.gras...@gmail.com on 30 Dec 2013 at 8:59

GoogleCodeExporter commented 8 years ago
Thanks Kim for the help.

Original comment by vsap...@gmail.com on 31 Dec 2013 at 4:19