[cfe-commits] r165091 - /cfe/trunk/lib/AST/MicrosoftMangle.cpp

[Nico Weber]

On Wed, Oct 3, 2012 at 9:40 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Wed, Oct 3, 2012 at 8:16 AM, Nico Weber <thakis at chromium.org> wrote:
>> On Wed, Oct 3, 2012 at 9:05 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> On Wed, Oct 3, 2012 at 2:12 AM, Nico Weber <nicolasweber at gmx.de> 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.