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



--- Comment #22 from monarchdodra@gmail.com 2013-05-29 12:36:54 PDT ---
(In reply to comment #21)
> 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.

Hum. OK. I see how that makes sense. One of the things that trips me up though is that D created the possibility to label a block but doesn't do anything with it.

To be honest, the way I had first understood labeling blocks was being able to
do this:
http://d.puremagic.com/issues/show_bug.cgi?id=8622

But even then, in that example, I would have expected a scope to be created.

-- 
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 #23 from Stewart Gordon <smjg@iname.com> 2013-05-29 13:57:52 PDT ---
(In reply to comment #22)
> Hum. OK. I see how that makes sense. One of the things that trips me up though is that D created the possibility to label a block but doesn't do anything with it.

The possibility to label a block is natural, because a block is a kind of statement.  What's odd is that adding a label alone turns it into a scopeless block.

> To be honest, the way I had first understood labeling blocks was being able to
> do this:
> http://d.puremagic.com/issues/show_bug.cgi?id=8622

I agree, that should work.  But the way it is at the moment, people who try it are likely to be bitten by bug 827.

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


Ali Cehreli <acehreli@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |acehreli@yahoo.com


--- Comment #24 from Ali Cehreli <acehreli@yahoo.com> 2013-05-29 14:03:07 PDT ---
Thanks for clarifying... :)

I can't see how this can be the intended behavior.

C labels do not expand the following scope. They are simply anchors for goto and switch-case statements.

D adds this cool feature of labeled scope, which has the unfortunate side effect of spilling the code of the block. I can't imagine how this has been the intention.

I propose changing the spec like this:

ScopeBlockOrNoScopeStatement:  <-- new
    NoScopeStatement
    ScopeBlockStatement

LabeledStatement:
    Identifier : ScopeBlockOrNoScopeStatement  <-- use the new one

NoScopeStatement:
    ;
    NonEmptyStatement
        <-- remove BlockStament to remove conflict with ScopeBlockStatement

That leaves the only other user of NoScopeStatement in question:

PragmaStatement:
    Pragma NoScopeStatement

I am not sure how removing BlockStament from NoScopeStatement will affect PragmaStatement.

Ali

-- 
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 #25 from Stewart Gordon <smjg@iname.com> 2013-05-29 14:23:04 PDT ---
(In reply to comment #24)
> I propose changing the spec like this:
> 
> ScopeBlockOrNoScopeStatement:  <-- new
>     NoScopeStatement
>     ScopeBlockStatement
> 
> LabeledStatement:
>     Identifier : ScopeBlockOrNoScopeStatement  <-- use the new one

How would all this differ from, let alone be an improvement over, simply changing LabeledStatement to

LabeledStatement:
    Identifier : Statement

?

> NoScopeStatement:
>     ;
>     NonEmptyStatement
>         <-- remove BlockStament to remove conflict with ScopeBlockStatement
> 
> That leaves the only other user of NoScopeStatement in question:
> 
> PragmaStatement:
>     Pragma NoScopeStatement

What about conditional compliation statements (debug, version, static if)? These are the primary reason NoScopeStatement exists in the first place.

-- 
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 #26 from Ali Cehreli <acehreli@yahoo.com> 2013-05-29 14:34:07 PDT ---
(In reply to comment #25)

> How would all this differ from, let alone be an improvement over, simply changing LabeledStatement to
>
> LabeledStatement:
>     Identifier : Statement

Sounds good. :)

> What about conditional compliation statements (debug, version, static if)? These are the primary reason NoScopeStatement exists in the first place.

I missed those because I made the mistake of searching only in the Statements page:

  http://dlang.org/statement.html

Ali

-- 
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 #27 from Nick Treleaven <ntrel-public@yahoo.co.uk> 2013-05-30 08:41:41 PDT ---
(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.

In order not to break code. I'm skeptical that a solution which causes any existing, debugged working code to silently break will be accepted, and I think that opinion makes sense.

If we are to avoid all possible code breakage, it's simple to just deprecate non-empty block statements after a label, since there are two workarounds (see comment 15) which do not require changing existing scope semantics. The deprecation message can advise the user of the workarounds.

I omitted to mention that an empty block statement after a label should still be allowed, as that is not bug-prone.

Maybe I wasn't clear - by deprecating I mean permanent deprecation without removal, essentially a warning that you can turn off if you really want to by silencing deprecation messages.

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

It's a trade off between either breaking working code or annoying users with deprecation messages that need small fixes to their code.

-- 
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 #28 from monarchdodra@gmail.com 2013-05-30 09:21:14 PDT ---
(In reply to comment #27)
> (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.
> 
> In order not to break code. I'm skeptical that a solution which causes any existing, debugged working code to silently break will be accepted, and I think that opinion makes sense.
> 
> If we are to avoid all possible code breakage, it's simple to just deprecate non-empty block statements after a label, since there are two workarounds (see comment 15) which do not require changing existing scope semantics. The deprecation message can advise the user of the workarounds.
> 
> I omitted to mention that an empty block statement after a label should still be allowed, as that is not bug-prone.
> 
> Maybe I wasn't clear - by deprecating I mean permanent deprecation without removal, essentially a warning that you can turn off if you really want to by silencing deprecation messages.
> 
> > This behavior was never according to spec anyways. I say we just fix it.
> 
> It's a trade off between either breaking working code or annoying users with deprecation messages that need small fixes to their code.

Just seems that because something is broken, your solution is to simply deprecate the entire feature. There is no reason to make labeling a block illegal or deprecated.

The correct solution would be to find a path that doesn't break code, but still marks invalid code as such, and give users a pre-emptive chance to fix the code, before it is definitely banned.

The (imo correct) path is that the NoScopeStatment should still provide no scope, but emit a deprecated message if anything declared in the scope is used. EG:

void main()
{
    label:
    {
        int i;
    }
    writeln(i); //(1)
}

(1) should emit a deprecation message.

-- 
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 #29 from Nick Treleaven <ntrel-public@yahoo.co.uk> 2013-05-30 09:29:04 PDT ---
(In reply to comment #28)
> (In reply to comment #27)
> > It's a trade off between either breaking working code or annoying users with deprecation messages that need small fixes to their code.
> 
> Just seems that because something is broken, your solution is to simply deprecate the entire feature. There is no reason to make labeling a block illegal or deprecated.
> 
> The correct solution would be to find a path that doesn't break code, but still marks invalid code as such, and give users a pre-emptive chance to fix the code, before it is definitely banned.

I didn't advocate banning it. I advocate permanent deprecation, because the semantics are bug prone, and we should (arguably) never risk *silently* breaking working code. There is no reason to remove it for a long time, possibly never.

> The (imo correct) path is that the NoScopeStatment should still provide no scope, but emit a deprecated message if anything declared in the scope is used. EG:
> 
> void main()
> {
>     label:
>     {
>         int i;
>     }
>     writeln(i); //(1)
> }
> 
> (1) should emit a deprecation message.

Sounds good. Can it be implemented easily?

-- 
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 #30 from monarchdodra@gmail.com 2013-05-30 09:42:02 PDT ---
(In reply to comment #29)
> (In reply to comment #28)
> > (In reply to comment #27)
> > > It's a trade off between either breaking working code or annoying users with deprecation messages that need small fixes to their code.
> > 
> > Just seems that because something is broken, your solution is to simply deprecate the entire feature. There is no reason to make labeling a block illegal or deprecated.
> > 
> > The correct solution would be to find a path that doesn't break code, but still marks invalid code as such, and give users a pre-emptive chance to fix the code, before it is definitely banned.
> 
> I didn't advocate banning it. I advocate permanent deprecation, because the semantics are bug prone, and we should (arguably) never risk *silently* breaking working code. There is no reason to remove it for a long time, possibly never.

I understand your position, but deprecation really implies that something WILL be removed, and usage NEEDS to stop. Having something permanently deprecated is not tenable: there is a compile mode that makes deprecated calls errors, and here, it is not an error.

In particular, since there are builders out there that compile with "deprecation is error", well end up breaking *valid* code, when trying to avoid not breaking broken code :/

> > The (imo correct) path is that the NoScopeStatment should still provide no scope, but emit a deprecated message if anything declared in the scope is used. EG:
> > 
> > void main()
> > {
> >     label:
> >     {
> >         int i;
> >     }
> >     writeln(i); //(1)
> > }
> > 
> > (1) should emit a deprecation message.
> 
> Sounds good. Can it be implemented easily?

That is a very good question :D

--------
Aren't we getting ahead of ourselves though? I think we should first concentrate on getting the spec changed, or at least, get Walter to agree that the current spec is not rational ?

Well, maybe he can be better persuaded if we present a non-breaking fix...

-- 
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 #31 from Stewart Gordon <smjg@iname.com> 2013-05-30 16:13:24 PDT ---
(In reply to comment #29)
> I didn't advocate banning it. I advocate permanent deprecation, because the semantics are bug prone, and we should (arguably) never risk *silently* breaking working code.

Your worry about this is out of all proportion, when you consider that:

(a) All the examples that have so far been posted here are contrived.  Somebody please show me some real-world code that only works as the programmer intended because of this "feature".  Surely there's far more real-world code that _doesn't_ behave as the programmer intended because of this.

(b) D isn't even an ISO (or ANSI or whatever) standard yet.  As such, it's
still a _good_ time to get all the potentially breaking changes out of the way.

(c) D has in its time had some breaking changes that are much more serious than
this.

(d) Even long-standardised programming languages have had breaking changes in their time.  C has had at least one breaking change in its time, though I don't know when this came in relation to its standardisation.  C++11 introduces a change to how template instantiations are lexed, which can cause code to behave differently.

> There is no reason to remove it for a long time,
> possibly never.
> 
>> The (imo correct) path is that the NoScopeStatment should still provide no scope, but emit a deprecated message if anything declared in the scope is used.

If you're going to go as far as that, why not emit a deprecation message if any declarations appear in it at all?

>> EG:
>> 
>> void main()
>> {
>>     label:
>>     {
>>         int i;
>>     }
>>     writeln(i); //(1)
>> }
>> 
>> (1) should emit a deprecation message.
> 
> Sounds good. Can it be implemented easily?

I can see that it would be complicated to get it right.  The tiny risk of silently breaking any real-world code doesn't seem to justify it.  As such, IMM, just fixing this issue and being done with it is the way to go.

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