bug in foreach continue

Hussien via Digitalmars-d-learn digitalmars-d-learn at puremagic.com
Fri Mar 17 06:53:58 PDT 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).






More information about the Digitalmars-d-learn mailing list