D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 602 - Compiler allows a goto statement to skip an initalization
Summary: Compiler allows a goto statement to skip an initalization
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: yebblies
URL: http://www.digitalmars.com/d/statemen...
Keywords: accepts-invalid, pull, TDPL
: 3549 4101 4667 6683 (view as issue list)
Depends on: 11562
Blocks: 2573
  Show dependency treegraph
 
Reported: 2006-11-26 08:18 UTC by Stewart Gordon
Modified: 2016-06-15 08:11 UTC (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Stewart Gordon 2006-11-26 08:18:35 UTC
The spec for GotoStatement states:

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

However, this code compiles:

----------
import std.stdio;

void main() {
    goto qwert;

    int yuiop = 42;

qwert:
    writefln(yuiop);
}
----------
4294112
----------

The spec doesn't comment on the use of goto to skip a declaration with no explicit initialization, but it has the same effect of bypassing the principle that all variables in D are initialized (unless this behaviour is overridden with a void).  In other words, this does the same:

----------
import std.stdio;

void main() {
    goto qwert;

    int yuiop;

qwert:
    writefln(yuiop);
}
----------

In both instances, a goto has been used to prevent a variable from being initialized.  Essentially, the compiler treats the code as being equivalent to:

----------
import std.stdio;

void main() {
    int yuiop = void;

    writefln(yuiop);
}
----------
Comment 1 Matti Niemenmaa 2007-03-30 13:06:45 UTC
Switch statements can also contain initializations prior to any case or default. This is either a symptom of this bug, or an enhancement request. I'll leave it here for now:

void main() {
	switch (1) {
		int x;
		case 1: assert (x == x.init);
	}
}
Comment 2 Matti Niemenmaa 2009-11-25 01:38:02 UTC
*** Issue 3549 has been marked as a duplicate of this issue. ***
Comment 3 Lars T. Kyllingstad 2010-08-17 09:51:02 UTC
*** Issue 4667 has been marked as a duplicate of this issue. ***
Comment 4 Kasumi Hanazuki 2011-10-26 12:06:07 UTC
*** Issue 6683 has been marked as a duplicate of this issue. ***
Comment 5 Martin Nowak 2013-10-31 10:15:09 UTC
*** Issue 4101 has been marked as a duplicate of this issue. ***
Comment 6 Martin Nowak 2013-10-31 10:31:38 UTC
Any ideas how to implement this? It seems like an algorithm to distinguish this is non-trivial.

I tested the following with clang and it gives me a correct warning (gcc 4.7.2 doesn't).
goto.c:3:9:warning: variable 'test' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]

int bug(int val)
{
    if (val)
        goto Lno;
    int test = 0;
 Lno: {}
    return test;
}
Comment 7 Stewart Gordon 2013-11-03 17:17:25 UTC
It seems to me quite simple: If there is any variable that is in scope at the point of the label but not at the point of the goto statement, then we have a problem.  Is there any case that this doesn't cover?
Comment 8 yebblies 2013-11-19 04:02:45 UTC
I have a fix for this but it requires all gotos to be resolved during semantic.

ie issue 11552
Comment 9 bearophile_hugs 2013-11-19 04:46:13 UTC
(In reply to comment #8)
> but it requires all gotos to be resolved during semantic.

Is it a problem for possible implementations of computed gotos?
Comment 10 Stewart Gordon 2013-11-19 05:02:45 UTC
(In reply to comment #9)
> (In reply to comment #8)
>> but it requires all gotos to be resolved during semantic.
> Is it a problem for possible implementations of computed gotos?

Since when has D (or any compiled language, for that matter) supported computed gotos?
Comment 11 yebblies 2013-11-19 05:04:20 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > but it requires all gotos to be resolved during semantic.
> 
> Is it a problem for possible implementations of computed gotos?

No.

As for this bug, I have implemented the semantics outlined in the specs for most statements:
"It is illegal for a GotoStatement to be used to skip initializations"

This means the following will be illegal:

void main() {
    goto label;
    int x;
    label: {}
}

Even though x is never referenced after the label.

This is probably going to break a bunch of code that doesn't actually do anything that bad.

An implementation that tracks the live ranges of variables is possible, but much more complicated.  Doing it this way would also allow catching 'used before set' bugs.
Comment 12 yebblies 2013-11-19 05:05:42 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> >> but it requires all gotos to be resolved during semantic.
> > Is it a problem for possible implementations of computed gotos?
> 
> Since when has D (or any compiled language, for that matter) supported computed
> gotos?

Maybe you've heard of gcc?

http://gcc.gnu.org/onlinedocs/gcc-3.1.1/gcc/Labels-as-Values.html
Comment 13 yebblies 2013-11-19 06:14:42 UTC
Some test cases, the second half are an enhancement.  Please feel free to add more in the same format.

static assert(!__traits(compiles, (bool b)
{
    if (b) goto label;
    int x;
    label: {}
    assert(!x);
}));

static assert(__traits(compiles, (bool b)
{
    int x;
    if (b) goto label;
    label: {}
}));

static assert(!__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
        int x;
    label: {}
        assert(!x);
    }
    catch
    {
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    label: {}
        int x;
    }
    catch
    {
    }
}));

static assert(!__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch
    {
        int x;
    label: {}
        assert(!x);
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch
    {
    label: {}
        int x;
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch(Exception)
    {
    label: {}
        int x;
    }
}));

static assert(!__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch(Exception e)
    {
    label: {}
        assert(e);
    }
}));

static assert(!__traits(compiles, (bool b)
{
    if (b) goto label;
    struct S
    {
        int x;
    }
    with (S())
    {
    label: {}
        assert(!x);
    }
}));

check it's ok if it's not used later

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    int x;
    label: {}
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
        int x;
    label: {}
    }
    catch
    {
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch
    {
        int x;
    label: {}
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    try
    {
    }
    catch(Exception e)
    {
    label: {}
    }
}));

static assert(__traits(compiles, (bool b)
{
    if (b) goto label;
    struct S
    {
        int x;
    }
    with (S())
    {
    label: {}
    }
}));
Comment 14 Stewart Gordon 2013-11-19 14:33:07 UTC
(In reply to comment #12)
> (In reply to comment #10)
>> Since when has D (or any compiled language, for that matter) 
>> supported computed gotos?
> 
> Maybe you've heard of gcc?
> 
> http://gcc.gnu.org/onlinedocs/gcc-3.1.1/gcc/Labels-as-Values.html

An interesting feature.  But even in C++ I can't see it working correctly, what with construction/destruction of objects that go into/out of scope to consider.  In D, this would apply to scope objects in the same way.

Remember that D supports goto only as a concession to programmers who want to break the rules of structured programming for efficiency reasons.  I think it goes without saying that the onus is on any D compiler writer who chooses to add such a feature as computed goto to figure out how to get all this working.

What next - somebody decides to create D extensions for writing self-modifying code?
Comment 15 bearophile_hugs 2013-11-19 15:32:19 UTC
(In reply to comment #14)

> I think it
> goes without saying that the onus is on any D compiler writer who chooses to
> add such a feature as computed goto to figure out how to get all this working.

You must leave the door open for future compiler writers that want to add computed gotos to D. They are an useful optimization if you want to implement threaded interpreters, certain finite state machines, etc. System languages as C/C++/D are the language you usually want to use if you want to write an interpreter that is both simple and fast.

Designing D (or its front-end implementation) to make it more difficult for future D compiler writers to implement compiled gotos (that are currently supported by the back-end of both GCC and Clang, so someone could add them to LDC2 or GDC without too much work) is silly.
Comment 16 Stewart Gordon 2013-11-19 16:25:39 UTC
(In reply to comment #15)
> Designing D (or its front-end implementation) to make it more difficult for
> future D compiler writers to implement compiled gotos (that are currently
> supported by the back-end of both GCC and Clang, so someone could add them to
> LDC2 or GDC without too much work) is silly.

Nobody here suggested designing D *to* make it more difficult for future D compiler writers to implement anything.  The point is that D has been designed to have initialisations and RAII, and as a side effect it is difficult to implement assigned/computed gotos.

Sacrificing these useful features just to make it easier to implement such a feature that will be rarely used (not least because of the inherent unportability resulting from leaving it to third-party extensions) would be silly.
Comment 17 bearophile_hugs 2013-11-19 16:54:20 UTC
(In reply to comment #16)

> D has been designed
> to have initialisations and RAII, and as a side effect it is difficult to
> implement assigned/computed gotos.

I think G++ and Clang support computed gotos in C++, that has RAII.

Generally when you use computed gotos, you are writing low level special code, so the programmer should be trusted. So some unsafety is acceptable when you use computed gotos.


> (not least because of the inherent
> unportability resulting from leaving it to third-party extensions)

I asked for computed gotos in D, and failing that I have asked for something less, to avoid the un-portability, I didn't even receive an answer from Walter:

http://www.digitalmars.com/d/archives/digitalmars/D/Explicitly_unimplemented_computed_gotos_122904.html
Comment 19 yebblies 2013-11-20 23:00:36 UTC
Related: issue 10524
Comment 20 github-bugzilla 2013-11-25 15:19:07 UTC
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/6448b41df1a22d7fc3eacae699c6c9ff619d18d7
Fix Issue 602 - Compiler allows a goto statement to skip an initalization

https://github.com/D-Programming-Language/dmd/commit/7360ae8611add4dc0a89cd870a6ac6490fb2a19b
Merge pull request #2839 from yebblies/issue602

Issue 602 - Compiler allows a goto statement to skip an initalization
Comment 21 bearophile_hugs 2013-12-01 05:22:11 UTC
This new change in the compiler forbids code like this that I have:


void main() {
    import std.stdio;
    goto LABEL;
    enum code = "writeln(10);";
    LABEL:
    mixin(code);
}


Is this bad/OK/good/acceptable?
Comment 22 yebblies 2013-12-01 05:33:39 UTC
(In reply to comment #21)
> This new change in the compiler forbids code like this that I have:
> 
> 
> void main() {
>     import std.stdio;
>     goto LABEL;
>     enum code = "writeln(10);";
>     LABEL:
>     mixin(code);
> }
> 
> 
> Is this bad/OK/good/acceptable?

The compiler is bad, the code is OK/good/acceptable.
Comment 23 bearophile_hugs 2013-12-01 05:36:58 UTC
(In reply to comment #22)

> The compiler is bad, the code is OK/good/acceptable.

So are you going to re-open this issue?
Comment 24 yebblies 2013-12-01 05:40:53 UTC
(In reply to comment #23)
> (In reply to comment #22)
> 
> > The compiler is bad, the code is OK/good/acceptable.
> 
> So are you going to re-open this issue?

No, this issue is fixed, it just caused a regression.  You can open a new issue for that is you like, but you don't have to.  I'll most likely do a patch for it tomorrow, unless someone beats me to it.
Comment 25 Martin Nowak 2013-12-01 11:40:00 UTC
(In reply to comment #24)
> You can open a new issue for that is you like, but you don't have to.

Tickets are always good to have (bug 11659).
Comment 26 github-bugzilla 2013-12-03 06:50:29 UTC
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/62a02a1a8a8ebe8d73d86e11064b01745cc38133
Fix cases where goto skips variable initialization found by fixing issue 602

https://github.com/D-Programming-Language/dmd/commit/b0e577bc2c330cbc9d936697ed31816c56c614da
Merge pull request #2918 from yebblies/ddmd602

[DDMD] Issue 602 cleanup
Comment 27 yebblies 2014-08-30 13:22:09 UTC
Issue 13321 has resulted in some of the cases regressing.
Comment 28 Kenji Hara 2015-02-23 05:02:24 UTC
(In reply to yebblies from comment #27)
> Issue 13321 has resulted in some of the cases regressing.

Some derived regressions are properly fixed now, so we can close this issue again.