http://d.puremagic.com/issues/show_bug.cgi?id=1041

           Summary: regression: incorrect code generation for scope(exit)
                    inside switch
           Product: DGCC aka GDC
           Version: 0.23
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: glue layer
        AssignedTo: dvdfrdmn@users.sf.net
        ReportedBy: thomas-dloop@kuehne.cn


# 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


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041


dvdfrdmn@users.sf.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dvdfrdmn@users.sf.net




------- Comment #1 from dvdfrdmn@users.sf.net  2007-03-10 16:32 -------
According to the D spec, the body of a SwitchStatement is ScopeStatement. Therefore, should not 'i'  be decremented after the switch?


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041





------- Comment #2 from thomas-dloop@kuehne.cn  2007-03-12 00:29 -------
I think one can interpret

# switch(3){
#    scope(exit) i--;
#    default:
#       ;
# }

as

# goto Ldefault;{
#    scope(exit) i--;
#    Ldefault:
#       ;
# }


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041


braddr@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |regression




-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041


kamm-removethis@incasoftware.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID




------- Comment #3 from kamm-removethis@incasoftware.de  2008-06-19 08:31 -------
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


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041


braddr@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |




------- Comment #4 from braddr@puremagic.com  2008-06-19 11:58 -------
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.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1041


Michael P <baseball.mjp@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |baseball.mjp@gmail.com


--- Comment #5 from Michael P <baseball.mjp@gmail.com> 2010-08-11 20:39:07 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #6 from Michael P <baseball.mjp@gmail.com> 2010-08-11 20:46:40 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #7 from Iain Buclaw <ibuclaw@ubuntu.com> 2010-08-12 11:36:49 PDT ---
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>:
#   {
#   }
# }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #8 from Iain Buclaw <ibuclaw@ubuntu.com> 2010-08-12 16:59:37 PDT ---
Created an attachment (id=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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------