Compiling foreach_bug.d produces an error, while compiling foreach_ok.d does not. IMHO that is a bug - it is clearly stated in current specification of The Language (http://www.digitalmars.com/d/statement.html#ForeachStatement) that variable can be declared outside.

Kind regards

Dejan

-- foreach_bug.d ---------8<------------------------------------------ import std.stdio;

int main(char[][] args) {
  char[] arg;
  foreach (arg; args)  // (*)
    writefln(arg);
  return 0;
}
// (*) Error: shadowing declaration foreach_bug.main.arg is deprecated

-- foreach_ok.d ----------8<------------------------------------------ import std.stdio;

int main(char[][] args) {
  foreach (char[] arg; args)
    writefln(arg);
  return 0;
}

-- version info ----------8<------------------------------------------
Digital Mars D Compiler v1.0
GCC 4.1.1
GNU/Linux (Fedora Core 6, linux 2.6.18)

Dejan Lekic wrote:
> Compiling foreach_bug.d produces an error, while compiling foreach_ok.d does
> not. IMHO that is a bug - it is clearly stated in current specification of The
> Language (http://www.digitalmars.com/d/statement.html#ForeachStatement) that
> variable can be declared outside.

No it's not. I just reread all of it. I didn't find any mention of that, nor a code sample that would support your case.
Furthermore, if it _did_ state that somewhere, it'd be a 'bug' in the specification, not in the compiler.

> -- foreach_bug.d ---------8<------------------------------------------
> import std.stdio;
> 
> int main(char[][] args) {
>   char[] arg;
>   foreach (arg; args)  // (*)
>     writefln(arg);
>   return 0;
> }
> // (*) Error: shadowing declaration foreach_bug.main.arg is deprecated

Frits van Bommel wrote:
> Dejan Lekic wrote:
>> Compiling foreach_bug.d produces an error, while compiling foreach_ok.d does
>> not. IMHO that is a bug - it is clearly stated in current specification of The
>> Language (http://www.digitalmars.com/d/statement.html#ForeachStatement) that
>> variable can be declared outside.
> 
> No it's not. I just reread all of it. I didn't find any mention of that, nor a code sample that would support your case.
> Furthermore, if it _did_ state that somewhere, it'd be a 'bug' in the specification, not in the compiler.
> 

The spec actually asserts the exact opposite (but you really have to look carefully to see it).


"At the start of each iteration, the variables _declared_ by the ForeachTypeList  are set to be a copy of the elements of the aggregate. If the variable is inout, it is a reference to the contents of that aggregate."

Based on this (note emphasis) the foreach is always declaring the variable. This can only be true if it is a new variable created at that point.

OTOH In many cases allowing exactly the usage you are trying for would make many things a lot easier. I would like to see it added.


int[] a = ...;
int e;

foreach(e_; a)
{
	if(0 == e_ % 2)
	{
		e = e_
		break;
	}
}

vs.

int[] a = ...;
int e;

foreach(e; a) if(0 == e % 2) break;


To avoid some bugs when doing this unintentionally, maybe require something like this:

int[] a = ...;
int e;

foreach(alias e; a) if(0 == e % 2) break;

Mr/Mrs BCS,
here is the part of documentation i maybe did not understand well:
----------------- snip ------------------
ForeachStatement:
    Foreach (ForeachTypeList; Aggregate) ScopeStatement

Foreach:
    foreach
    foreach_reverse

ForeachTypeList:
    ForeachType
    ForeachType , ForeachTypeList

ForeachType:
    inout Type Identifier
    Type Identifier
    inout Identifier
    Identifier
----------------- snip ------------------

If I understand this correctly, than simple code like the one in foreach_bug.d should work because 'arg' is clearly an 'Identifier' from the 'ForeachType' rule above.

I am posting foreach_bug.d again here:
-- foreach_bug.d ---------8<------------------------------------------
import std.stdio;

int main(char[][] args) {
  char[] arg;
  foreach (arg; args)  // <-- problem here
    writefln(arg);
  return 0;
}

Dejan Lekic wrote:
> Mr/Mrs BCS,
> here is the part of documentation i maybe did not understand well:
> ----------------- snip ------------------
> ForeachStatement:
>     Foreach (ForeachTypeList; Aggregate) ScopeStatement
> 
> Foreach:
>     foreach
>     foreach_reverse
> 
> ForeachTypeList:
>     ForeachType
>     ForeachType , ForeachTypeList
> 
> ForeachType:
>     inout Type Identifier
>     Type Identifier
>     inout Identifier
>     Identifier
> ----------------- snip ------------------
> 
> If I understand this correctly, than simple code like the one in foreach_bug.d
> should work because 'arg' is clearly an 'Identifier' from the 'ForeachType' rule
> above.
> 
> I am posting foreach_bug.d again here:
> -- foreach_bug.d ---------8<------------------------------------------
> import std.stdio;
> 
> int main(char[][] args) {
>   char[] arg;
>   foreach (arg; args)  // <-- problem here
>     writefln(arg);
>   return 0;
> }

This is a case where D applies implicit typing rules to determine the type of a variable declaration. The Above syntax definition says that an identifier can be place at that point, it says nothing about what is done with it. In this case, the semantics cause it to be a variable deceleration.

BCS wrote:
> Frits van Bommel wrote:
>> Dejan Lekic wrote:
>>> Compiling foreach_bug.d produces an error, while compiling foreach_ok.d does
>>> not. IMHO that is a bug - it is clearly stated in current specification of The
>>> Language (http://www.digitalmars.com/d/statement.html#ForeachStatement) that
>>> variable can be declared outside.
>>
>> No it's not. I just reread all of it. I didn't find any mention of that, nor a code sample that would support your case.
>> Furthermore, if it _did_ state that somewhere, it'd be a 'bug' in the specification, not in the compiler.
>>
> 
> The spec actually asserts the exact opposite (but you really have to look carefully to see it).
> 
[snip]
> To avoid some bugs when doing this unintentionally, maybe require something like this:
> 
> int[] a = ...;
> int e;
> 
> foreach(alias e; a) if(0 == e % 2) break;

Not a bad idea.  I can't even think of anything better than 'alias' to use for it. Perhaps there should be an enhancement request filed on the bugzilla?

-- Chris Nicholson-Sauls

Dejan Lekic wrote:
> Mr/Mrs BCS,
> here is the part of documentation i maybe did not understand well:
> ----------------- snip ------------------
> ForeachStatement:
>     Foreach (ForeachTypeList; Aggregate) ScopeStatement
> 
> Foreach:
>     foreach
>     foreach_reverse
> 
> ForeachTypeList:
>     ForeachType
>     ForeachType , ForeachTypeList
> 
> ForeachType:
>     inout Type Identifier
>     Type Identifier
>     inout Identifier
>     Identifier
> ----------------- snip ------------------
> 
> If I understand this correctly, than simple code like the one in foreach_bug.d
> should work because 'arg' is clearly an 'Identifier' from the 'ForeachType' rule
> above.

I'm pretty sure that if you look only at the syntax definition, the following (full) program is also valid:

-----
void something(X Y)	         // X undefined
{
	blaat() + "yeah";        // blaat undefined
	Z = 1.234;               // Z undefined
	int b = [1, "foo", .2];  // init value is invalid array
                                 // literal, and certainly not an int
}
-----

It's only when you try to make sense of it that you realize that even though it's syntactically correct, it's full of semantic errors.
In fact, I just ran that file through 'dmd -v' and it parses fine. The semantic pass is where it errors out. Same as your example, though yours doesn't error out until the third semantic pass instead of giving up in the first one.