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

[jahanian]

Patch LGTM, AFAICT. I checked it in r148359 with couple of changes.
I don't see any need to generate body of folded case label block. gcc doesn't do that and
we generally try to stay with gcc's behavior in such undocumented cases.
Also, I modified the test case to be clang friendly.

- Fariborz

On Jan 16, 2012, at 8:06 PM, Aaron Ballman wrote:

> On Mon, Jan 16, 2012 at 5:48 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Mon, Jan 16, 2012 at 1:12 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>> On Mon, Jan 16, 2012 at 10:26 AM, jahanian <fjahanian at apple.com> wrote:
>>>> 
>>>> On Jan 16, 2012, at 10:20 AM, Eli Friedman wrote:
>>>> 
>>>> On Mon, Jan 16, 2012 at 10:14 AM, Aaron Ballman <aaron at aaronballman.com>
>>>> wrote:
>>>> 
>>>> On Mon, Jan 16, 2012 at 11:56 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>>> 
>>>> 
>>>> I'll come up with some test cases to check this out tonight, and pass
>>>> 
>>>> them along to verify I understand the concerns properly.
>>>> 
>>>> 
>>>> Okay... I'm specifically concerned about the code generation for
>>>> non-constant-folded-switch containing a constant-folded switch.
>>>> 
>>>> 
>>>> Just committed a test case which seems to handle this correctly.
>>> 
>>> Managed to come up with the testcase I was thinking of:
>>> 
>>> #include <stdio.h>
>>> int test(int val){
>>>  switch (val) {
>>>  case 4:
>>>    do {
>>>      switch (6) {
>>>        case 6: do { case 5: printf("bad\n"); } while (0);
>>>      };
>>>    } while (0);
>>>  }
>>>  return 0;
>>> }
>>> 
>>> int main(void) {
>>>  return test(5);
>>> }
>> 
>> I just had the chance to try out this test case, and you're right, it
>> does produce the wrong code in this instance.  It emits the case 5
>> against the outer switch (because SwitchInsn is non-null).
> 
> I've attached a new patch and test case which should address this issue.
> 
> HTH!
> 
> ~Aaron
> <switch_folding.patch>