[llvm-commits] [llvm] r137830 - in /llvm/trunk: include/llvm/MC/ lib/Target/ARM/Disassembler/ lib/Target/MBlaze/Disassembler/ lib/Target/X86/Disassembler/ test/MC/Disassembler/ARM/ tools/llvm-mc/ utils/TableGen/

[Jim Grosbach]

On Sep 1, 2011, at 10:25 AM, David Blaikie wrote:

> On Thu, Sep 1, 2011 at 9:38 AM, Jim Grosbach <grosbach at apple.com> wrote:
>> 
>> Hi James,
>> 
>> This looks a lot better, thank you.
>> 
>> 
>>> +static bool Check(MCDisassembler::DecodeStatus &Out, MCDisassembler::DecodeStatus In) {
>>> +  switch (In) {
>>> +    case MCDisassembler::Success:
>>> +      // Out stays the same.
>>> +      return true;
>>> +    case MCDisassembler::SoftFail:
>>> +      Out = In;
>>> +      return true;
>>> +    case MCDisassembler::Fail:
>>> +      Out = In;
>>> +      return false;
>>> +
>>> +    default: assert(0);
>> 
>> Should be llvm_unreachable(), not assert(0), since that's what it's semantically expressing.
> 
> I believe the usual approach in the LLVM code base is to omit the
> default case if you have covered all the enumeration values and rely
> on the compiler to warn if any case is omitted. By adding the default
> case you thwart this warning.

Also totally reasonable. I'm fine either way, personally. :)

-Jim