Bug 1041 – regression: incorrect code generation for scope(exit) inside switch

# int main(){ # int i = 2; # # switch(3){ # scope(exit) i--; # # default: # } # # if(i != 2){ # assert(0); # } # # return 0; # } gdmd-0.23 run/s/switch_22_B.d -ofx && ./x Error: AssertError Failure run/s/switch_22_B.d(22) test case: http://dstress.kuehne.cn/run/s/switch_22_B.d

According to the D spec, the body of a SwitchStatement is ScopeStatement. Therefore, should not 'i' be decremented after the switch?

I think one can interpret # switch(3){ # scope(exit) i--; # default: # ; # } as # goto Ldefault;{ # scope(exit) i--; # Ldefault: # ; # }

I don't see how this can be a bug, the spec says switch statements introduce a new scope and dmd and gdc behave accordingly. This fails, as expected: switch(1) { int i; } i++; // undefined identifier: i

Your example doesn't match the reported problem at all. The variable in the original example is at a higher scope, thus visible within the switch statement.

The problem appears to be in DefaultStatement::toIR in d-glue.cc. The error, "default cannot be in different try block level from switch" is handled in dmd in s2ir.c, in DefaultStatement::toIR. DMD: void DefaultStatement::toIR(IRState *irs) { Blockx *blx = irs->blx; block *bcase = blx->curblock; block *bdefault = irs->getDefaultBlock(); block_next(blx,BCgoto,bdefault); list_append(&bcase->Bsucc,blx->curblock); if (blx->tryblock != irs->getSwitchBlock()->Btry) error("default cannot be in different try block level from switch"); incUsage(irs, loc); if (statement) statement->toIR(irs); } GDC: void DefaultStatement::toIR(IRState * irs) { irs->doCase(NULL_TREE, cblock); if (statement) statement->toIR( irs ); } You can see that GDC does not do the checks that DMD does. At the moment, I'm not sure how the DMD code translates to what should be in GDC.

Very closely related bug with case statements. Changing the line "default:" to "case 3:" produces the error "case cannot be in different try block level from switch" in dmd, but again no error in gdc. The code for handling this is in CaseStatement::toIR in dmd, but there is nothing to check for that in gdc.

To be honest I was kinda unsure about this, as I did generally agree that this should be valid for the reason that the body of a SwitchStatement is ScopeStatement. And although the code itself compiles it down to Figure 1 (which eventually gets optimised to just 'i = i - 1;' anyway.), apparently it should be treated more like Figure 2. Which has my hands half in the air in agreeing that it (the current behaviour) probably shouldn't be the defined behaviour of the program. What made me committed was this isn't seen in GCC-3.4. So at the very least, need to fix up some bits to get some consistency across the board. Figure 1: # switch (3) { # default: goto <A>; # } # { # <A>: # { # try { # } # finally { # i = i - 1; # } # } # } Figure 2: # switch (3) { # try { # default: goto <A>: # } # finally { # i = i - 1; # } # } # { # <A>: # { # } # }

Created attachment 715 quick hack for issue 1041 Attaching a my quick rudimentary kludge. It's not pretty to my eyes, but it does the job. What is does, is save the currentScope of SwitchStatement::toIR to irs->switchStatement. Then when it enters DefaultStatement or CaseStatement, tests that irs->switchStatement is the parentScope of the itself. This requires a little extra work in TryFinallyStatement::toIR, I'm happy with the new code I've added to it, with the exception that I'm rather unsure about explicitly calling startScope and endScope for it to work correctly with it. (Although the documentation does say try executes a new ScopeStatement). Micheal, if you could review it for me - maybe take inspiration and think of a better approach, would be appreciated. :-) Thanks.

Just to note before I'm off, no you cannot build phobos using the above.

Well, the error in std/string.d is quite odd, because it has nothing to do with switch statements. Commenting out the code in ifind in the else statement to look like this: (Look lines 352-357) /*foreach (ptrdiff_t i, dchar c2; s) { c2 = std.uni.toUniLower(c2); if (c1 == c2) return i; }*/ Makes it compile string.d okay. The next problems found in std/regexp.d can be simplified to this: void main() { switch( 1 ) { case 1: switch(1) { default: } break; } } Basically: nested switch statements seem to be a problem with the patch.

Created attachment 716 issue 1041 attempt #2 Second round, works without frizzles. Though I'd still prefer it if you'd take it with a pinch of salt. :-) IMO, I think it would be better if we'd implement a smarter framework for testing which scope we are in. But I'll leave that with you to decide Michael.

Fixed. Still uses some chicanery to get the job done. But a little better at doing it. See hg commit 197 / release 0.25