On Wed, Oct 3, 2012 at 8:50 AM, Nico Weber <[email protected]> wrote: > On Wed, Oct 3, 2012 at 9:40 PM, Aaron Ballman <[email protected]> wrote: >> On Wed, Oct 3, 2012 at 8:16 AM, Nico Weber <[email protected]> wrote: >>> On Wed, Oct 3, 2012 at 9:05 PM, Aaron Ballman <[email protected]> >>> wrote: >>>> On Wed, Oct 3, 2012 at 2:12 AM, Nico Weber <[email protected]> wrote: >>>>> Author: nico >>>>> Date: Wed Oct 3 01:12:27 2012 >>>>> New Revision: 165091 >>>>> >>>>> URL: http://llvm.org/viewvc/llvm-project?rev=165091&view=rev >>>>> Log: >>>>> Replace a default: with an explicit list of cases. No functionality >>>>> change. >>>>> >>>>> Modified: >>>>> cfe/trunk/lib/AST/MicrosoftMangle.cpp >>>>> >>>>> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp >>>>> URL: >>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=165091&r1=165090&r2=165091&view=diff >>>>> ============================================================================== >>>>> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original) >>>>> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Wed Oct 3 01:12:27 2012 >>>>> @@ -810,7 +810,12 @@ >>>>> break; >>>>> } >>>>> /* fallthrough */ >>>>> - } default: { >>>>> + } >>>>> + case TemplateArgument::Template: >>>>> + case TemplateArgument::TemplateExpansion: >>>>> + case TemplateArgument::Declaration: >>>>> + case TemplateArgument::NullPtr: >>>>> + case TemplateArgument::Pack: { >>>>> // Issue a diagnostic. >>>>> DiagnosticsEngine &Diags = Context.getDiags(); >>>>> unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, >>>> >>>> Now if another value is added to the list, it will silently fail; is >>>> this acceptable? I would feel more comfortable if there was still a >>>> default case that would be marked as unreachable. >>> >>> It won't silently fail, clang will warn that a enum case isn't handled. >> >> Clang will, but not all compilers will. MSVC doesn't warn on that >> situation, for instance. Marking default with llvm_unreachable solves >> that by ensuring there's no warning, and by complaining loudly if the >> default is ever hit (so you can't ignore it). > > Right, that way it's only caught at runtime, not compile time. Isn't that > worse? > > From what I can tell, exhaustive switches without defaults are very > common in clang's codebase.
I stand corrected -- we turn this feature *on* in clang with -w14062 as an additional option. So this will fire by default in MSVC. I'd say your change looks great then! ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
