Jump to page: 1 2 3
Thread overview
bug in foreach continue
Mar 17, 2017
Hussien
Mar 17, 2017
Adam D. Ruppe
Mar 17, 2017
Hussien
Mar 17, 2017
Jonathan M Davis
Mar 17, 2017
Michael
Mar 17, 2017
Jonathan M Davis
Mar 17, 2017
Hussien
Mar 17, 2017
Adam D. Ruppe
Mar 17, 2017
Hussien
Mar 17, 2017
Adam D. Ruppe
Mar 17, 2017
H. S. Teoh
Mar 17, 2017
Ali Çehreli
Mar 19, 2017
H. S. Teoh
Mar 19, 2017
Mike Parker
Mar 19, 2017
Jonathan M Davis
Mar 17, 2017
Hussien
Mar 21, 2017
Stefan Koch
Mar 22, 2017
Jesse Phillips
Mar 23, 2017
H. S. Teoh
Mar 23, 2017
Ali Çehreli
Mar 23, 2017
Adam D. Ruppe
Mar 24, 2017
H. S. Teoh
Mar 17, 2017
Adam D. Ruppe
Mar 17, 2017
Daniel Kozak
March 17, 2017
foreach (y; aliasSeqOf!["a", "b", "c"])
{
static if (y == "a") { }
else
 pragma(msg, y);
}

works but

foreach (y; aliasSeqOf!["a", "b", "c"])
{
static if (y == "a") continue
pragma(msg, y);
}

fails.

Seems like continue needs to be static aware.

This leads to subtle bugs where one thinks the continue statement(like in in a runtime foreach loop, should jump to to the next iteration, but it doesn't.

The runtime versions of the code work identically, and so should the static versions.

March 17, 2017
On Friday, 17 March 2017 at 01:34:52 UTC, Hussien wrote:
> Seems like continue needs to be static aware.

That's not a bug, pragma is triggered when the code is *compiled*, not when it is run. The code is compiled, even if it is skipped over by a continue.
March 17, 2017
On Friday, 17 March 2017 at 01:41:47 UTC, Adam D. Ruppe wrote:
> On Friday, 17 March 2017 at 01:34:52 UTC, Hussien wrote:
>> Seems like continue needs to be static aware.
>
> That's not a bug, pragma is triggered when the code is *compiled*, not when it is run. The code is compiled, even if it is skipped over by a continue.

My point is, that it shouldnt! a static if should have a static continue in some way. It is confusing to be thinking in terms of the compile time code and (static if) and use a ambiguous statement(continue) and it not be compile time.

Either one needs a static version of continue or a warning when used inside a static if.

As is, continue doesn't even apply to the static case, so it shouldn't be allowed. A static foreach with a dynamic continue is illogical yet no warning is emitted.

March 17, 2017
Dne 17.3.2017 v 02:34 Hussien via Digitalmars-d-learn napsal(a):

> foreach (y; aliasSeqOf!["a", "b", "c"])
> {
> static if (y == "a") { }
> else
>  pragma(msg, y);
> }
>
> works but
>
> foreach (y; aliasSeqOf!["a", "b", "c"])
> {
> static if (y == "a") continue
> pragma(msg, y);
> }
>
> fails.
>
> Seems like continue needs to be static aware.
>
> This leads to subtle bugs where one thinks the continue statement(like in in a runtime foreach loop, should jump to to the next iteration, but it doesn't.
>
> The runtime versions of the code work identically, and so should the static versions.

No it is not a bug, it works as I would expected:
import std.meta;
import std.stdio;

void main()
{
    foreach (y; aliasSeqOf!(["a", "b", "c"]))
    {
        static if (y == "a") continue;
        else writeln(y);
        pragma(msg, y);
    }
}


March 17, 2017
On Friday, March 17, 2017 01:55:19 Hussien via Digitalmars-d-learn wrote:
> On Friday, 17 March 2017 at 01:41:47 UTC, Adam D. Ruppe wrote:
> > On Friday, 17 March 2017 at 01:34:52 UTC, Hussien wrote:
> >> Seems like continue needs to be static aware.
> >
> > That's not a bug, pragma is triggered when the code is *compiled*, not when it is run. The code is compiled, even if it is skipped over by a continue.
>
> My point is, that it shouldnt! a static if should have a static continue in some way. It is confusing to be thinking in terms of the compile time code and (static if) and use a ambiguous statement(continue) and it not be compile time.
>
> Either one needs a static version of continue or a warning when used inside a static if.
>
> As is, continue doesn't even apply to the static case, so it shouldn't be allowed. A static foreach with a dynamic continue is illogical yet no warning is emitted.

I tend to agree with this. If the foreach is static, and continue and break are just going to be ignored, then they should just be illegal. Allowing them is just going to confuse people. Now, making it so that they actually work statically has some interesting possibilities, but that would fall apart as soon as you have any constructs that would use continue or break (e.g. a loop or switch statement) inside the static foreach, and it might break code in rare cases. So, we're probably better off just making them illegal. But having them be legal just seems disingenious, since they don't do anything.

- Jonathan M Davis

March 17, 2017
On Friday, 17 March 2017 at 11:30:48 UTC, Jonathan M Davis wrote:
> On Friday, March 17, 2017 01:55:19 Hussien via Digitalmars-d-learn wrote:
>
> I tend to agree with this. If the foreach is static, and continue and break are just going to be ignored, then they should just be illegal. Allowing them is just going to confuse people. Now, making it so that they actually work statically has some interesting possibilities, but that would fall apart as soon as you have any constructs that would use continue or break (e.g. a loop or switch statement) inside the static foreach, and it might break code in rare cases. So, we're probably better off just making them illegal. But having them be legal just seems disingenious, since they don't do anything.
>
> - Jonathan M Davis

What exactly IS happening in the case of a continue in a static-if? I could sort of imagine that maybe if you were expecting the loop to be unrolled, that you then have a continue statement in the correct part of the unrolled loop. But I take it this isn't what's happening?
March 17, 2017
On Friday, March 17, 2017 11:53:41 Michael via Digitalmars-d-learn wrote:
> On Friday, 17 March 2017 at 11:30:48 UTC, Jonathan M Davis wrote:
> > On Friday, March 17, 2017 01:55:19 Hussien via Digitalmars-d-learn wrote:
> >
> > I tend to agree with this. If the foreach is static, and continue and break are just going to be ignored, then they should just be illegal. Allowing them is just going to confuse people. Now, making it so that they actually work statically has some interesting possibilities, but that would fall apart as soon as you have any constructs that would use continue or break (e.g. a loop or switch statement) inside the static foreach, and it might break code in rare cases. So, we're probably better off just making them illegal. But having them be legal just seems disingenious, since they don't do anything.
> >
> > - Jonathan M Davis
>
> What exactly IS happening in the case of a continue in a static-if? I could sort of imagine that maybe if you were expecting the loop to be unrolled, that you then have a continue statement in the correct part of the unrolled loop. But I take it this isn't what's happening?

Okay. I completely misunderstood what was happening based on the previous posts. For instance, from this test

import std.meta;
import std.stdio;

void main()
{
    foreach(str; ["hello", "world", "foo", "bar"])
    {
        foreach(i; AliasSeq!(0, 1, 2, 3, 4))
        {
            static if(i / 2 == 0)
                writefln("%s: %s", i, str);
            else
            {
                writefln("%s: skipping %s", i, str);
                break;
            }
        }
        writeln();
    }
}

it looks like break and continue _are_ used at compile time, since it prints

0: hello
1: hello
2: skipping hello

0: world
1: world
2: skipping world

0: foo
1: foo
2: skipping foo

0: bar
1: bar
2: skipping bar

whereas if the break were just compiled in to be run at runtime, you never would have seen any of the strings past "hello" in the outer loop get printed. So, the issue is _not_ that continue and break are always treated as runtime constructs or outright ignored. And if we have

import std.meta;
import std.stdio;

void main()
{
    foreach(str; AliasSeq!("a", "b", "c"))
    {
        static if(str == "a")
            continue;
        writeln(str);
    }
}

it prints

b
c

because when the loop is unrolled, wirteln isn't compiled in (and in fact, if you compile with -w, the code won't compile at all, because writeln is unreachable in the "a" iteration).

On the other hand, if we have

import std.meta;
import std.stdio;

void main()
{
    foreach(str; AliasSeq!("a", "b", "c"))
    {
        static if(str == "a")
            continue;
        pragma(msg, str);
    }
}

then we get

a
b
c

which is what the OP was complaining about. Now, as to whether that's a bug or not, I don't know. pragma(msg, ...) prints whenever it's compiled, and as shown by the compilation error when you compile in the -w case, the code after the static if _is_ compiled in. It just ends up not being run. Certainly, the continue and break are _not_ skipped like I originally understood to be the case. So, that's not the bug. If there _is_ a bug, it's that the rest of the code in the loop after the static if is compiled after the continue, and I don't know that that is a bug. Remember that this is basically doing loop unrolling, which is not necessarily the same thing as you would expect from a "static foreach." It's not like we're building a string mixin here. It's more like the same code is being copy-pasted over and over but potentially with some tweaks on a given iteration.

So, I'm not at all convinced that this is a bug, but I do agree that the semantics are a bit wonky. Regardless, based on what happens with the writeln case, it's clearly the case that you should be using an else branch if you want to cleanly use a continue in a foreach like this.

- Jonathan M Davis

March 17, 2017
On Friday, 17 March 2017 at 13:10:23 UTC, Jonathan M Davis wrote:
> On Friday, March 17, 2017 11:53:41 Michael via Digitalmars-d-learn wrote:
>> On Friday, 17 March 2017 at 11:30:48 UTC, Jonathan M Davis wrote:
>> > On Friday, March 17, 2017 01:55:19 Hussien via Digitalmars-d-learn wrote:
>> >
>> > I tend to agree with this. If the foreach is static, and continue and break are just going to be ignored, then they should just be illegal. Allowing them is just going to confuse people. Now, making it so that they actually work statically has some interesting possibilities, but that would fall apart as soon as you have any constructs that would use continue or break (e.g. a loop or switch statement) inside the static foreach, and it might break code in rare cases. So, we're probably better off just making them illegal. But having them be legal just seems disingenious, since they don't do anything.
>> >
>> > - Jonathan M Davis
>>
>> What exactly IS happening in the case of a continue in a static-if? I could sort of imagine that maybe if you were expecting the loop to be unrolled, that you then have a continue statement in the correct part of the unrolled loop. But I take it this isn't what's happening?
>
> Okay. I completely misunderstood what was happening based on the previous posts. For instance, from this test
>
> import std.meta;
> import std.stdio;
>
> void main()
> {
>     foreach(str; ["hello", "world", "foo", "bar"])
>     {
>         foreach(i; AliasSeq!(0, 1, 2, 3, 4))
>         {
>             static if(i / 2 == 0)
>                 writefln("%s: %s", i, str);
>             else
>             {
>                 writefln("%s: skipping %s", i, str);
>                 break;
>             }
>         }
>         writeln();
>     }
> }
>
> it looks like break and continue _are_ used at compile time, since it prints
>
> 0: hello
> 1: hello
> 2: skipping hello
>
> 0: world
> 1: world
> 2: skipping world
>
> 0: foo
> 1: foo
> 2: skipping foo
>
> 0: bar
> 1: bar
> 2: skipping bar
>
> whereas if the break were just compiled in to be run at runtime, you never would have seen any of the strings past "hello" in the outer loop get printed. So, the issue is _not_ that continue and break are always treated as runtime constructs or outright ignored. And if we have
>

Yes, but you have a nested foreach loop. One runtime and one compile time. The break goes with the runtime loop... but NORMAL programming logic tells us that the break goes with the loop that it exists in.

>     foreach(str; ["hello", "world", "foo", "bar"])
>     {
>         foreach(i; AliasSeq!(0, 1, 2, 3, 4))
>         {
>             static if(i / 2 == 0)
>                 writefln("%s: %s", i, str);
>             else
>             {
>                 writefln("%s: skipping %s", i, str);
>                 break;
>             }
>         }
>         writeln();
>     }


reduces too

>     foreach(str; ["hello", "world", "foo", "bar"])
>     {
          writefln("%s: %s", 0, str);
          writefln("%s: %s", 1, str);
          writefln("%s: skipping %s", 2, str);
          break;

          writefln("%s: skipping %s", 3, str);
          break;

          writefln("%s: %s", 4, str);
          break;

>         writeln();
>     }

And I'm sure this is not what you intended(The first break is good enough). Not only does the unrolled loop not make sense, the standard logic any normal programmer without knowing the ambiguity of foreach loop in D would assume that the break in the inner most loop is for the inner most foreach.

This can create very complex and subtle bugs. It gets worse the more complex the mixing of the runtime and compile time programming is.

Static foreach loops should use static break and static continue... "Birds of a feather flock together"...

If one needs to use a runtime break in a compile time loop, then some special mechanism could be added to disambiguate the two.

Mixing compile time and runtime identifiers is a very bad idea! It creates very subtle ambiguities that will cause very serious bugs in the future when D becomes more widely used. At that point, it will probably be too late to be changed(as it will then break code that took in to account the ambiguity).

One can say that all break's and continues are runtime... which is how it currently is. That is fine, but a warning should be issued because it goes against meta programming logic. When I think of meta programming, I don't start thinking using a completely different set of rules... especially since most of the syntax and semantics is between runtime and compile time is identical.







> import std.meta;
> import std.stdio;
>
> void main()
> {
>     foreach(str; AliasSeq!("a", "b", "c"))
>     {
>         static if(str == "a")
>             continue;
>         writeln(str);
>     }
> }
>
> it prints
>
> b
> c
>
> because when the loop is unrolled, wirteln isn't compiled in (and in fact, if you compile with -w, the code won't compile at all, because writeln is unreachable in the "a" iteration).
>
> On the other hand, if we have
>
> import std.meta;
> import std.stdio;
>
> void main()
> {
>     foreach(str; AliasSeq!("a", "b", "c"))
>     {
>         static if(str == "a")
>             continue;
>         pragma(msg, str);
>     }
> }
>
> then we get
>
> a
> b
> c
>
> which is what the OP was complaining about. Now, as to whether that's a bug or not, I don't know. pragma(msg, ...) prints whenever it's compiled, and as shown by the compilation error when you compile in the -w case, the code after the static if _is_ compiled in. It just ends up not being run. Certainly, the continue and break are _not_ skipped like I originally understood to be the case. So, that's not the bug. If there _is_ a bug, it's that the rest of the code in the loop after the static if is compiled after the continue, and I don't know that that is a bug. Remember that this is basically doing loop unrolling, which is not necessarily the same thing as you would expect from a "static foreach." It's not like we're building a string mixin here. It's more like the same code is being copy-pasted over and over but potentially with some tweaks on a given iteration.
>
> So, I'm not at all convinced that this is a bug, but I do agree that the semantics are a bit wonky. Regardless, based on what happens with the writeln case, it's clearly the case that you should be using an else branch if you want to cleanly use a continue in a foreach like this.
>
> - Jonathan M Davis

A bug or not is irrelevant. There is an issue with the design of for loops between runtime and compile time versions. There is overlap in syntax and it creates ambiguity and this leads to REAL bugs... sometimes very hard to track down. When someone is scanning a complex piece of code and they run across something similar to the above. They are not going to think of the continue and break's as being dynamic and having complex fall through behavior.

They are going to expect them to continue or break out of the inner most loop that they are called from. That is the definition of them:

"A break statement results in the termination of the statement to which it applies ( switch , for , do , or while ). A continue statement is used to end the current loop iteration and return control to the loop statement."

"The break statement "jumps out" of a loop.
The continue statement "jumps over" one iteration in the loop."


It's bad enough foreach is ambiguous... we can't now if it's static or not unless we analyze the object it loops over.

A better syntax would be to introduce a static syntax for static constructs:

$foreach = static/compile time foreach
$continue = static/compile time continue
$break = static/compile time break
$if = static/compile time if
$while
$do
void $foo(int) = static/compile time function. (not necessarily needed because of CTFE but it is explicit.


Such a syntax(not saying that $ should be used, just had to use something for demonstration purposes) is not ambiguous. When someone sees $continue, they know EXACTLY what it is doing.

What has been done is this:

Black:

1.
of the very darkest color owing to the absence of or complete absorption of light; the opposite of white.
"black smoke"
synonyms:	dark, pitch-black, jet-black, coal-black, ebony, sable, inky
"a black horse"
antonyms:	white

    (of the sky or night) completely dark owing to nonvisibility of the sun, moon, or stars.
    "the sky was moonless and black"
    synonyms:	unlit, dark, starless, moonless, wan; More
    literarytenebrous, Stygian
    "a black night"
    antonyms:	clear, bright
    deeply stained with dirt.
    "his clothes were absolutely black"
    (of a plant or animal) dark in color as distinguished from a lighter variety.
    "Japanese black pine"
    synonyms:	dark, pitch-black, jet-black, coal-black, ebony, sable, inky
    "a black horse"
    antonyms:	white
    (of coffee or tea) served without milk or cream.
    of or denoting the suits spades and clubs in a deck of cards.
    (of a ski run) of the highest level of difficulty, as indicated by black markers positioned along it.

2.
of any human group having dark-colored skin, especially of African or Australian Aboriginal ancestry.


And what will eventually have to happen is that the term black will have to be banned as a racist word... simply because of the ambiguity. It is like all words, people are confused by them because of their different meanings. It's best to be explicit as necessary, and in this case, I think D is not explicit enough. The convenience factor created by ambiguity will cause more problems than it solves down the road for a select group of people.

A non-ambiguous syntax could be added on top of what currently exists and eventually the old syntax could be depreciated... If continue/break are the only problems then some other, simpler solution, could be found. I think a warning would be good enough.

e.g., "Warning: continue used in static foreach loop."

which brings attention to the issue. Since no one likes warnings, a pragma(-warn, C34324) could be used(or whatever D's mechanism is to specific appropriate warnings).




March 17, 2017
On Friday, 17 March 2017 at 13:10:23 UTC, Jonathan M Davis wrote:
> it looks like break and continue _are_ used at compile time, since it prints

They are working exactly the same way as any other loop. The fact that it is unrolled and the dead code removed from the binary is an implementation detail that doesn't change the semantics of the loop, just like any other loop unroll optimization.

`continue` is more like `goto` than it is like `static if`, even when looping over AliasSeq.

> whereas if the break were just compiled in to be run at runtime, you never would have seen any of the strings past "hello" in the outer loop get printed.

No, they did a normal break at runtime. But they are breaking the INNER loop, so the outer loop continues. The generated code simply removed the dead portions since the optimizer realized there was no point carrying it around.

You are totally overthinking this, loops still work the same way regardless of what they are iterating over.

The only special thing about an AliasSeq thing is that the current value is available at compile time, so it is legal to use in more places (then the compiler implementation generates the code differently to make it happen, but that's the same idea as passing -funroll-loops or -O3 and getting different code, they don't change how break works.)

> Remember that this is basically doing loop unrolling, which is not necessarily the same thing as you would expect from a "static foreach." It's not like we're building a string mixin here.

Yes.
March 17, 2017
On Friday, 17 March 2017 at 13:53:58 UTC, Hussien wrote:
> Yes, but you have a nested foreach loop. One runtime and one compile time. The break goes with the runtime loop... but NORMAL programming logic tells us that the break goes with the loop that it exists in.

It did. It broke the loop counting the numbers, but continued the loop of strings normally.


People have requested a `static foreach` before, but that request has not yet been granted.

> They are going to expect them to continue or break out of the inner most loop that they are called from.

That is exactly what they do, and exactly what happened in all the examples posted.
« First   ‹ Prev
1 2 3