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

[David Blaikie]

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.

- David