[cfe-commits] r148243 - /cfe/trunk/lib/CodeGen/CGStmt.cpp

[Aaron Ballman]

On Mon, Jan 16, 2012 at 11:56 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jan 16, 2012 at 9:49 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Mon, Jan 16, 2012 at 9:35 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>> Author: fjahanian
>>> Date: Mon Jan 16 11:35:57 2012
>>> New Revision: 148243
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=148243&view=rev
>>> Log:
>>> Fixes a crash during constant folding of a switch and case
>>> statement which has an unscoped case inside it.
>>> Patch by Aaron Ballman
>>>
>>> Modified:
>>>    cfe/trunk/lib/CodeGen/CGStmt.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=148243&r1=148242&r2=148243&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Mon Jan 16 11:35:57 2012
>>> @@ -878,6 +878,16 @@
>>>  }
>>>
>>>  void CodeGenFunction::EmitCaseStmt(const CaseStmt &S) {
>>> +  // If there is no enclosing switch instance that we're aware of, then this
>>> +  // case statement and its block can be elided.  This situation only happens
>>> +  // when we've constant-folded the switch, are emitting the constant case,
>>> +  // and part of the constant case includes another case statement.  For
>>> +  // instance: switch (4) { case 4: do { case 5: } while (1); }
>>> +  if (!SwitchInsn) {
>>> +    EmitStmt(S.getSubStmt());
>>> +    return;
>>> +  }
>>
>> I'm not convinced that this patch does the right thing if the given
>> construct is nested into another switch.
>
> Ah - good point. I'd only thought about having the constant folding
> stuff remove the case labels for cleanliness - but it might be a
> matter of correctness too, as you've pointed out/hinted at. (yay for
> the 'nice' thing being the right thing too)
>
> I was/am meaning to look at this alternative approach, but since it
> may be a matter of correctness, perhaps Aaron will be motivated to
> test/investigate/fix it if I don't get around to it.

I'll come up with some test cases to check this out tonight, and pass
them along to verify I understand the concerns properly.

~Aaron