https://issues.dlang.org/show_bug.cgi?id=24534

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |accepts-invalid

--

https://issues.dlang.org/show_bug.cgi?id=24534

Ben <ben.james.jones@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ben.james.jones@gmail.com

--- Comment #1 from Ben <ben.james.jones@gmail.com> ---
*** Issue 24535 has been marked as a duplicate of this issue. ***

--

https://issues.dlang.org/show_bug.cgi?id=24534

Richard Cattermole <alphaglosined@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3
                 CC|                            |alphaglosined@gmail.com
           Severity|normal                      |enhancement

--- Comment #2 from Richard Cattermole <alphaglosined@gmail.com> ---
After thinking about this, it's intentional.

This allows you to have multiple blocks at the end of your function that handle things like success versus error.

It is very C style, and is used in dmd.

The solution of course would be to create a new scope so it doesn't bleed between the labels.

```d
goto Lerror;

Lsuccess:
{

}

Lerror:
{

}
```

If this is fixed, it's going to need a very long deprecation.

Or we could kill off this logic entirely and have type state analysis proper which works on a per-variable basis rather than a coarse reverse search which it is currently.

--

https://issues.dlang.org/show_bug.cgi?id=24534

--- Comment #3 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
Well, it's in clear violation of the spec: https://dlang.org/spec/statement.html#goto-statement

So, either the spec is wrong, or dmd is.

Also, if you slap @safe on main, it compiles even though the initialization is skipped, which should never happen in @safe code. So, there's a bug here of some kind regardless.

--

https://issues.dlang.org/show_bug.cgi?id=24534

--- Comment #4 from Ben <ben.james.jones@gmail.com> ---
Gotta be a bug:

void f2(){ //compiles fine
    int x2;
    goto Label2;
    Dummy2:
    int y2;
    Label2:
    int z2;
}


GotoStatement::semantic() for Label2
goto lastvar: x2
LabelStatement::semantic() for Dummy2
labelstatement lastvar: x2
LabelStatement::semantic() for Label2
labelstatement lastvar: x2 <--- WRONG, should be y2

--

https://issues.dlang.org/show_bug.cgi?id=24534

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #5 from Dlang Bot <dlang-bot@dlang.rocks> ---
@benjones updated dlang/dmd pull request #16510 "fix issue24534 : goto can skip declarations in labeled statements without error" fixing this issue:

- Fix Bugzilla Issue 24534 : goto can skip declarations in labeled statements without error

https://github.com/dlang/dmd/pull/16510

--

https://issues.dlang.org/show_bug.cgi?id=24534

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |normal

--- Comment #6 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
Since this is in violation of the spec, I'm changing this back to a bug. If it's decided that the current behavior is desirable for whatever reason, then the spec will need to be updated instead. And even if the spec is updated instead of making this code illegal like the spec says it should be, the fact that @safe works with this code in spite of the fact that the variable is not properly initialized means that there's a compiler bug either way. So, it's not an enhancement.

--

https://issues.dlang.org/show_bug.cgi?id=24534

--- Comment #7 from Richard (Rikki) Andrew Cattermole <alphaglosined@gmail.com> ---
It is a bug in the compiler, I agree on that too.

But, it is also desirable to skip variable declarations, as long as you don't touch them you do not violate the spec clause.

Since this is used in C code quite often, and therefore ported code (such as
dmd's backend).

A shortcut was taken with the current reverse search to analyze it, and it doesn't work as a result.

Solving this properly requires type state analysis as that is what this particular clause involves.

Therefore any attempts to fix this that does not do type state analysis is unfortunately another shortcut.

--

https://issues.dlang.org/show_bug.cgi?id=24534

--- Comment #8 from Richard (Rikki) Andrew Cattermole <alphaglosined@gmail.com> ---
I've done some playing around to see if backward goto's have an equivalent issue.

It seems dmd is actually smart there, and I couldn't get that to not work.

Generates a try/finally:

```
Label:
    SomethingWithSideEffects var = 0;
    try
    {
        if (uniform01() > 0.5)
            return 0;
        goto Label;
    }
    finally
        var.~this();
```

This means only forward goto's have broken analysis.

--

https://issues.dlang.org/show_bug.cgi?id=24534

--- Comment #9 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
(In reply to Richard (Rikki) Andrew Cattermole from comment #7)
> But, it is also desirable to skip variable declarations, as long as you don't touch them you do not violate the spec clause.

The spec clause specifically disallows that:

"It is illegal for a GotoStatement to be used to skip initializations."

There's certainly an argument to be made that the rule could be more fine-grained and require that no variable be accessed whose declaration was skipped (which would also mean that the variable couldn't have a destructor), but as the rule currently stands, you can't legally skip variable declarations whether the variable is then used or not.

--