[llvm-commits] [llvm] r123297 - /llvm/trunk/lib/Target/ARM/ARMMCCodeEmitter.cpp

[Matt Beaumont-Gay]

On Tue, Jan 11, 2011 at 23:42, Chris Lattner <clattner at apple.com> wrote:
>
> On Jan 11, 2011, at 6:13 PM, Nick Lewycky wrote:
>
> On 11 January 2011 17:47, Chris Lattner <clattner at apple.com> wrote:
>>
>> On Jan 11, 2011, at 5:42 PM, Matt Beaumont-Gay wrote:
>>
>> > Author: matthewbg
>> > Date: Tue Jan 11 19:42:42 2011
>> > New Revision: 123297
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=123297&view=rev
>> > Log:
>> > Prefer llvm_unreachable to assert(0)
>>
>> Hi Matt,
>>
>> Please use assert(0) unless we need the "does not fallthrough" behavior of
>> llvm_unreachable.
>
> The difference is that llvm_unreachable still generates code in a Release
> build while assert() doesn't, right?
> Matt made this commit because we were seeing:
>   llvm/lib/Target/ARM/ARMMCCodeEmitter.cpp:653: error: control reaches end
> of non-void function [-Wreturn-type]
> so it sounds like llvm_unreachable isn't the right tool, but we'd like to
> fix this that doesn't generate warnings in release builds.
>
> Please move the default case to the first part of the switch statement:
> switch (whatever) {
> default:  assert(0 && ...
> case 0:
> that way, in non-assert builds, the compiler still thinks everything is
> great.

Got it, done in r123317. Is it worth mentioning this in the coding style docs?

-Matt