www.digitalmars.com         C & C++   DMDScript  

D.gnu - [Issue 1041] New: regression: incorrect code generation for scope(exit) inside switch

reply d-bugmail puremagic.com writes:
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


-- 
Mar 08 2007
next sibling parent d-bugmail puremagic.com writes:
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?


-- 
Mar 10 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
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:
#       ;
# }


-- 
Mar 11 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041


braddr puremagic.com changed:

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




-- 
Apr 13 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
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


-- 
Jun 19 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
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.


-- 
Jun 19 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Aug 11 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Aug 11 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Aug 12 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Aug 12 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #9 from Iain Buclaw <ibuclaw ubuntu.com> 2010-08-13 00:32:59 PDT ---
Just to note before I'm off, no you cannot build phobos using the above.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #10 from Michael P <baseball.mjp gmail.com> 2010-08-13 13:24:01 PDT
---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 13 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041



--- Comment #11 from Iain Buclaw <ibuclaw ubuntu.com> 2010-08-14 02:11:55 PDT
---
Created an attachment (id=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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 14 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041


Iain Buclaw <ibuclaw ubuntu.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #716 is|1                           |0
              patch|                            |
 Attachment #716 is|0                           |1
           obsolete|                            |


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 16 2010
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1041


Iain Buclaw <ibuclaw ubuntu.com> changed:

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


--- Comment #12 from Iain Buclaw <ibuclaw ubuntu.com> 2010-08-16 18:19:14 PDT
---
Fixed. Still uses some chicanery to get the job done. But a little better at
doing it.

See hg commit 197 / release 0.25

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