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



--- Comment #12 from monarchdodra@gmail.com 2013-05-29 03:53:27 PDT ---
(In reply to comment #11)
> (In reply to comment #10)
> 
> > - Potentially creates different behavior from C code.
> 
> What are the conditions for it to create different behaviour? Maybe you can show two programs, in C and D, that behave differently.

This program for example, will behave differently in C and D:

--------
--------
int i = 3;

void main()
{
  {
    int some_condition = 1;
    if ( some_condition )
      goto block_end;

    /* dummy code */
  } block_end:

  {
    int i = 7;
    printf("%i", i); //Local i
  }

  printf("%i", i); //Global i (?)
}
--------
C prints: "70"
D prints: "77"
--------

The conditions needed to create it is mostly just un-expected shadowing. This one shows difference with C, but even without C, the behavior is not what would have been expected from a D developer anyway.

The issue can also be reproduced in member functions, that have local variables that shadow members. The shadowing will outlive its scope, and create unexpected behavior.

-- 
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=199



--- Comment #13 from bearophile_hugs@eml.cc 2013-05-29 04:12:54 PDT ---
(In reply to comment #12)

> int i = 3;
> 
> void main()
> {
>   {
>     int some_condition = 1;
>     if ( some_condition )
>       goto block_end;
> 
>     /* dummy code */
>   } block_end:
> 
>   {
>     int i = 7;
>     printf("%i", i); //Local i
>   }
> 
>   printf("%i", i); //Global i (?)
> }
> --------
> C prints: "70"
> D prints: "77"
> --------


With GCC this C code gives me "73":
http://codepad.org/7uKsouJ0
http://ideone.com/GBiRCX


//import core.stdc.stdio: printf;
#include "stdio.h"

int i = 3;

int main() {
    {
        int some_condition = 1;
        if (some_condition)
            goto block_end;

        /* dummy code */
    } block_end:

    {
        int i = 7;
        printf("%i", i); //Local i
    }

    printf("%i", i); // Global i (?)
    return 0;
}

-- 
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=199



--- Comment #14 from monarchdodra@gmail.com 2013-05-29 04:57:14 PDT ---
(In reply to comment #13)
> With GCC this C code gives me "73":

Yes, my bad. Copy paste typo.

-- 
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=199


Nick Treleaven <ntrel-public@yahoo.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ntrel-public@yahoo.co.uk


--- Comment #15 from Nick Treleaven <ntrel-public@yahoo.co.uk> 2013-05-29 05:21:55 PDT ---
comment #1:
> Adding '{}' immediately after the label seems to be a viable workaround.

Or moving the label inside the block:

  {
    label:
    /* code */
  }

Walter Bright, comment #6:
> I don't want to change this because it could break existing code

We could deprecate having a BlockStatement after a label, with a message suggesting to move the label within the block. That way no code gets broken.

-- 
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=199



--- Comment #16 from monarchdodra@gmail.com 2013-05-29 06:27:45 PDT ---
(In reply to comment #15)
> Walter Bright, comment #6:
> > I don't want to change this because it could break existing code
> 
> We could deprecate having a BlockStatement after a label, with a message suggesting to move the label within the block. That way no code gets broken.

Wait. What? Deprecate having a block statement after a label? Why would we do that? That's completely arbitrary. Plus, that would *definitely* break more code than what we'd break with a straight up fix.

This behavior was never according to spec anyways. I say we just fix it.

-- 
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=199



--- Comment #17 from Stewart Gordon <smjg@iname.com> 2013-05-29 08:01:42 PDT ---
(In reply to comment #10)
> According to this talk: http://forum.dlang.org/thread/ohctkdnupmbprglobwtc@forum.dlang.org#post-axonofactkyzpagilcbm:40forum.dlang.org The spec does NOT state it should work that way.

Yes it does, as I quoted in comment 5.

LabeledStatement:
    Identifier : NoScopeStatement

But since
- it's arbitrary
- it's counter-intuitive
- I can't see how any code can take advantage of this "feature"
I agree in principle that it should be changed to simply

LabeledStatement:
    Identifier : Statement

While this would change the behaviour of code such as that in comment 13, ISTM far more likely that anybody who seriously writes such code is unfamiliar with this arcane detail of the D spec.

(In reply to comment #16)
> (In reply to comment #15)
>> Walter Bright, comment #6:
>>> I don't want to change this because it could break existing code
>> 
>> We could deprecate having a BlockStatement after a label, with a message suggesting to move the label within the block. That way no code gets broken.
> Wait. What? Deprecate having a block statement after a label? Why would we do that? That's completely arbitrary. Plus, that would *definitely* break more code than what we'd break with a straight up fix.

My inkling is that, compared with the amount of code it would break, it would cause far more code that was already broken to generate a compiler error.

-- 
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=199



--- Comment #18 from monarchdodra@gmail.com 2013-05-29 10:17:39 PDT ---
(In reply to comment #17)
> (In reply to comment #10)
> > According to this talk: http://forum.dlang.org/thread/ohctkdnupmbprglobwtc@forum.dlang.org#post-axonofactkyzpagilcbm:40forum.dlang.org The spec does NOT state it should work that way.
> 
> Yes it does, as I quoted in comment 5.
> 
> LabeledStatement:
>     Identifier : NoScopeStatement

That was my statement :D but it was rebuked by Ali in comment 7 that:

It is still a bug because NoScopeStatement does not mean "expand into current scope." It means "do not introduce a scope" and clearly allows blocked statements:

NoScopeStatement:
    ;
    NonEmptyStatement
    BlockStatement


> (In reply to comment #16)
> > (In reply to comment #15)
> >> Walter Bright, comment #6:
> >>> I don't want to change this because it could break existing code
> >> 
> >> We could deprecate having a BlockStatement after a label, with a message suggesting to move the label within the block. That way no code gets broken.
> > Wait. What? Deprecate having a block statement after a label? Why would we do that? That's completely arbitrary. Plus, that would *definitely* break more code than what we'd break with a straight up fix.
> 
> My inkling is that, compared with the amount of code it would break, it would cause far more code that was already broken to generate a compiler error.

Isn't "code that was broken now generates a compiler error" good though...?

I still maintain that the amount of use case in nature is probably trivially low. Of those, the amount it would break even lower. I have no proof, but D is modern, and modern tends to stay clear away of goto and labels anyways.

-- 
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=199



--- Comment #19 from Stewart Gordon <smjg@iname.com> 2013-05-29 11:30:27 PDT ---
(In reply to comment #18)
> It is still a bug because NoScopeStatement does not mean "expand into current scope." It means "do not introduce a scope"

What is the distinction you are making here?

> Isn't "code that was broken now generates a compiler error" good though...?

It is, and that's part of the point I was trying to make.

-- 
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=199



--- Comment #20 from monarchdodra@gmail.com 2013-05-29 11:38:54 PDT ---
(In reply to comment #19)
> (In reply to comment #18)
> > It is still a bug because NoScopeStatement does not mean "expand into current scope." It means "do not introduce a scope"
> 
> What is the distinction you are making here?

Well, if it was a "ScopedStatement", it would have meant that:

-----
label: int i; //i is scoped here
i = 5; //Error, i has not been declared
-----

In contrast, with a NoScopedStatement, all it means is that when you write:
-----
label: "{stuff}"
-----
It means that that "{stuff}" itself is not scoped: the "{stuff}" statement can be seen from the outside. But that doesn't mean the "{stuff}" itself doesn't create its own internal scope, which contains "stuff", and which can't be seen from the outside...

That's how I read it now, and the explanation Ali was making.

> > Isn't "code that was broken now generates a compiler error" good though...?
> 
> It is, and that's part of the point I was trying to make.

Ok, cool. Since it was a reply to me, it seemed you were arguing against it.

-- 
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=199



--- Comment #21 from Stewart Gordon <smjg@iname.com> 2013-05-29 12:05:24 PDT ---
The only statement nodes that create a scope according to the spec are ScopeStatement and ScopeBlockStatement.  When you have

Identifier : { StatementList }

the structure is

LabeledStatement
    Identifier
    :
    NoScopeStatement
        BlockStatement
            {
            StatementList
            }

No node that creates a scope here.  It's the same way with the conditional compilation statements.  Of course, statements within the StatementList may introduce their own scopes, but nothing in this parse tree as it stands creates a scope.

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