Bug 24754 – cannot take address of a member array in a ref foreach

The following code should compile: ``` struct Foo { int[] foo; @safe void foobar() { int* f; foreach(ref e; foo) f = &e; } } void main() { Foo foo; foo.foobar(); } ``` Shouldn't yield: ``` Up to 2.078.3: Failure with output: onlineapp.d(10): Error: cannot take address of local e in @safe function foobar Since 2.079.1: Failure with output: onlineapp.d(10): Error: cannot take address of local `e` in `@safe` function `foobar` ```

Pointers are not allowed in @safe functions at all. That's not a bug, it's a feature!

This might be a bug, DIP1000 should be catching this I think. I checked, and with it turned on it did not catch it. But yes, a pointer is allowed in @safe functions, including taking a pointer of currently.

> This might be a bug, DIP1000 should be catching this I think. If `&e` is the address of an int inside the `Foo.foo` array, then that is fine so long as the array is null or GC allocated. However, if Foo.foo is changed to `int[1] foo;`, it still won't error and it should: ```d import std.stdio; struct Foo { int[1] foo; @safe int* foobar() { int* f; foreach(ref e; foo) f = &e; // escaping address of foo[0] return f; } } void main() @safe { int* p; { Foo foo = Foo([1]); //writeln(&foo.foo[0]); p = foo.foobar(); } p.writeln(); (*p).writeln(); } ```

> Pointers are not allowed in @safe functions at all. What are you talking about? They are, its specified in the spec. 20.24.1. Safe Functions. Please don't close tickets as invalid as you see them and just because you think they are invalid. It's not productive for anyone. Maybe someone else sympathize with the same problem, you might not have enough understanding of it, ask for elaboration, or present arguments against it... --- > That's not a bug, it's a feature! This is a bug, and an industry facing bug. Probably one of the reasons no one at industry use or rely on it (e.g. like people do in Rust), in practice, @safe, is because its buggy and have holes. Being restrictive at the point of not allowing pointers is not a real feature, if D didn't allow pointers. It would certainly make the feature even less adopted. It's another question if we would discuss if D allows a subset of pointers only (like ref variables, or like Zig does, define the pointer semantic restrictions in the type), but that goes out of scope both from what is defined in spec and what we have today. Also if pointers was the real problem, explain why this code compiles: ``` struct Foo { int[] foo; @safe void foobar() { int* f; foreach(i; 0 .. foo.length) f = &foo[i]; } } void main() { Foo foo; foo.foobar(); } ``` It uses pointers, doesn't it?

> If `&e` is the address of an int inside the `Foo.foo` array, then that is fine so long as the array is null or GC allocated. Specification guarantees that only safe assignments are made, so, whatever is assigned to `foo`, unless its `= void` assignment, its safe to de-reference it, again, as long as the assignment is safe. > However, if Foo.foo is changed to `int[1] foo;`, it still won't error and it should The code you presented escapes variables. The code I presented doesn't escape anything beyond its own scope, which is conceptually not escaping. Escaping is defined via DIP1000, and I'm not expecting the code to compile when escaping without defining `return` attribute to the member function.

> > However, if Foo.foo is changed to `int[1] foo;`, it still won't error and it should Also the code you presented has a different type, even though trivially valid if you don't escape from within the function, which you do, so it doesn't apply to this particular issue. --- For the frontend folks, just ease this from DIP1000 preview flag, it doesn't make sense to be within the flag.

Funny enough, if you compile the initial code snippet with `-vcg-ast` (that yields incorrect D code, but you can understand how it converts it to), it pretty much converts it to something compatible like this, which is compilable: ``` struct Foo { int[] foo; void foobar() { int* f = null; { int[] __r2 = this.foo[]; ulong __key3 = 0LU; for (; __key3 < __r2.length; __key3 += 1LU) { int* e = &__r2[__key3]; f = e; } } } } ```

Reduces to: ```D void main() @safe { foreach(ref e; new int[1]) int* f = &e; } ```

(In reply to Dennis from comment #8) > Reduces to: > > ```D > void main() @safe > { > foreach(ref e; new int[1]) > int* f = &e; > } > ``` Thanks! So, maybe this should also be valid, but more arguable if requires dip1000 internal checks the same way with `ref`, although, spec-wise it should: ``` void main() @safe { foreach(e; new int[1]) int* f = &e; } ``` I removed the `ref`, `f` doesn't escape its own scope, the same way this should compile: ``` void main() @safe { int e; int* f = &e; } ```

(In reply to Luís Ferreira from comment #4) > > Pointers are not allowed in @safe functions at all. > > What are you talking about? They are, its specified in the spec. 20.24.1. > Safe Functions. Sorry. I misread your code. And of course the compiler could allow for some more patterns to be considered @safe, but I thought that was intentional so. E.g. the following (a simple self-reference check) is also not considered @safe and I was told that this is intentional without DIP1000: ``` struct Foo { Foo opAssign(const ref Foo f) { if(&f == this) return this; ... } } ``` I'm glad if this is considered a design-flaw that should be fixed.

> the same way this should compile > I'm glad if this is considered a design-flaw that should be fixed. Without `-preview=dip1000` taking the address of local, stack allocated variables is not @safe. This is not going to be fixed, we're not going to add special cases to allow a halfway, in between form of safe stack pointers. It's all or nothing. The question here is whether a `ref` variable counts as a local variable. `ref` parameters freely bind to any lvalue in @safe code, including local variables, so the answer there is yes. A ref loop variable in foreach is a bit more difficult, as it depends on the type of the thing you're iterating over. In theory, in the absence of -preview=dip1000, it can't be a stack pointer when you're iterating over a dynamic array, so this issue's test case can be made `@safe` on the basis of that. On the other hand, there's still the gaping hole that local static arrays can be sliced in `@safe` code without `-preview=dip1000`, and get an unrestricted lifetime. So fixing this issue facilitates more exploitation of that hole (issue 24750). So perhaps we need to make that a deprecation first and then consider adding a special for this, but whether that's worth it depends on the future direction of dip1000 vs Robert's Simple Safe D.

(In reply to Dennis from comment #8) > Reduces to: > > ```D > void main() @safe > { > foreach(ref e; new int[1]) > int* f = &e; > } > ``` The compiler doesn't know (without optimization or -dip1000) that the program is equivalent to that. It doesn't know below that `f` doesn't escape the scope of foobar somehow. @safe void foobar() { int* f; foreach(ref e; foo) f = &e; } So it seems counter-intuitive to the way D's semantic analysis works to allow this case without -dip1000.

(In reply to Nick Treleaven from comment #3) > @safe > int* foobar() > { > int* f; > foreach(ref e; foo) > f = &e; // escaping address of foo[0] > return f; > } BTW with dmd git master and -dip1000, the static array escape is now caught: Error: scope variable `f` may not be returned

(In reply to Nick Treleaven from comment #12) > The compiler doesn't know (without optimization or -dip1000) that the > program is equivalent to that. It could, all it needs to know is that typeof(Foo.foo) == typeof(new int[1]) == int[]. > It doesn't know below that `f` doesn't escape > the scope of foobar somehow. Doesn't mattter, `f` may escape, unless it's a `scope int[]`, which doesn't really exist without `-preview=dip1000`.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/18267 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB