On Sunday, 22 September 2019 at 17:26:20 UTC, Mike Franklin wrote:
> Interestingly, taking the address of a `ref` in `@safe` code is prevented, but only when compiling *without* -preview=dip1000.  See https://run.dlang.io/is/GArFzk
>
> That's seems like a bug to me.

That's just the normal pre-DIP-1000 error you get when taking the address of any local (`ref` or not). Allowing `&some_local` is pretty much the point of DIP 1000, so it's expected that the old error doesn't appear anymore.

There is a bug for sure, though. If `ref` implies `scope`, there should be a new error ("cannot take address of scope parameter"). If `scope` is not implied, it's ok that the function compiles, but then it shouldn't be possible to call it on a local.

On Sunday, 22 September 2019 at 17:17:40 UTC, Mike Franklin wrote:
> Yes, `scope ref` is definitely a thing.  Compare the following.
>
> [...]
>
> This was taken from the discussion at https://forum.dlang.org/post/twgsrfcolypurgylhizh@forum.dlang.org

Huh.

I knew that was a thing, but I assumed the semantics were emergent. I didn't realize they might be intentional.

On Sunday, 22 September 2019 at 17:54:48 UTC, ag0aep6g wrote:
> There is a bug for sure, though. If `ref` implies `scope`, there should be a new error ("cannot take address of scope parameter"). If `scope` is not implied, it's ok that the function compiles, but then it shouldn't be possible to call it on a local.

It's allowed as long as you don't escape... wait, no, it's always allowed. Whoops. The following code compiles with -dip1000.

    @safe:

    int* foo(ref int x)
    {
        int* a = &x;
        return a;
    }

    void main() {
        int* p;
        {
            int x;
            p = foo(x);
        }
        *p = 1;			// Memory corruption
    }

That's a bug.

On Monday, 23 September 2019 at 08:46:18 UTC, Olivier FAURE wrote:
> Whoops. The following code compiles with -dip1000.
>
>     @safe:
>
>     int* foo(ref int x)
>     {
>         int* a = &x;
>         return a;
>     }
>
>     void main() {
>         int* p;
>         {
>             int x;
>             p = foo(x);
>         }
>         *p = 1;			// Memory corruption
>     }
>
> That's a bug.

Well, dip1000 doesn't do data-flow analyses. Which means the compiler doesn't see that `x` escapes through `a`.

On Monday, 23 September 2019 at 18:39:03 UTC, Sebastiaan Koppe wrote:
> On Monday, 23 September 2019 at 08:46:18 UTC, Olivier FAURE wrote:
>> Whoops. The following code compiles with -dip1000.
>>
>>     @safe:
>>
>>     int* foo(ref int x)
>>     {
>>         int* a = &x;
>>         return a;
>>     }
>>
>>     void main() {
>>         int* p;
>>         {
>>             int x;
>>             p = foo(x);
>>         }
>>         *p = 1;			// Memory corruption
>>     }
>>
>> That's a bug.
>
> Well, dip1000 doesn't do data-flow analyses. Which means the compiler doesn't see that `x` escapes through `a`.

AFAICT, according to dip1000 this code should not allow the result of `foo(x)` to be assigned to p, as it has a longer lifetime. Likewise, it should not allow foo to return &x without the parameter being annotated with `return`. This looks like a bug in the implementation.

On Monday, 23 September 2019 at 18:46:40 UTC, Meta wrote:
> AFAICT, according to dip1000 this code should not allow the result of `foo(x)` to be assigned to p, as it has a longer lifetime. Likewise, it should not allow foo to return &x without the parameter being annotated with `return`. This looks like a bug in the implementation.

Specifically, this breaks rule 1 from DIP1000:

A scope variable can only be initialized and assigned from values that have lifetimes longer than the variable's lifetime. (As a consequence a scope variable can only be assigned to scope variables that have shorter lifetime.)

This rule is broken by `p = foo(x)`, since p has a longer lifetime than x. Marking the `ref int x` parameter of foo `scope`, `return scope` or `scope return` did not help, but marking it as `return` *did* cause the compiler to correctly reject this code. Also changing foo to:

int* foo(ref int x)
{
    return &x;
}

OR

int* foo(int* x)
{
    return x;
}
...
p = foo(&x);

Causes the compiler to correctly reject this code, so the problem seems to specifically be with returning the address of a ref parameter stored in a local variable.

On Monday, 23 September 2019 at 18:46:40 UTC, Meta wrote:
> On Monday, 23 September 2019 at 18:39:03 UTC, Sebastiaan Koppe
>> Well, dip1000 doesn't do data-flow analyses. Which means the compiler doesn't see that `x` escapes through `a`.
>
> AFAICT, according to dip1000 this code should not allow the result of `foo(x)` to be assigned to p, as it has a longer lifetime. Likewise, it should not allow foo to return &x without the parameter being annotated with `return`.

It does all those things when you replace the body of foo with just `return &x;`.

On Monday, 23 September 2019 at 19:11:46 UTC, Sebastiaan Koppe wrote:
> On Monday, 23 September 2019 at 18:46:40 UTC, Meta wrote:
>> On Monday, 23 September 2019 at 18:39:03 UTC, Sebastiaan Koppe
>>> Well, dip1000 doesn't do data-flow analyses. Which means the compiler doesn't see that `x` escapes through `a`.
>>
>> AFAICT, according to dip1000 this code should not allow the result of `foo(x)` to be assigned to p, as it has a longer lifetime. Likewise, it should not allow foo to return &x without the parameter being annotated with `return`.
>
> It does all those things when you replace the body of foo with just `return &x;`.

Not really. When you do that, DMD rejects the `return` statement, not the assignment `p = foo(x);`.

It works as expected when you mark the parameter with `return`. This has nothing to do with missing flow analysis. It's just a bug.

Maybe there is some clever idea regarding `ref` parameters and the implementation is just buggy, or maybe the bug is that they're not treated just like pointers.

I'd suggest treating them just like pointers. Seems to me that's the most straight-forward approach.

Meaning:

----
@safe:

int* fp(int* p) { return p; } /* This works. */
int* fr(ref int r)  { return &r; } /* So this should work too. */

void main()
{
    int x;
    fp(&x); /* This doesn't work. */
    fr(x); /* So this shouldn't work either. */
}
----

On Monday, 23 September 2019 at 20:50:30 UTC, ag0aep6g wrote:
> On Monday, 23 September 2019 at 19:11:46 UTC, Sebastiaan Koppe wrote:
>> On Monday, 23 September 2019 at 18:46:40 UTC, Meta wrote:
>>> On Monday, 23 September 2019 at 18:39:03 UTC, Sebastiaan Koppe
>>>> Well, dip1000 doesn't do data-flow analyses. Which means the compiler doesn't see that `x` escapes through `a`.
>>>
>>> AFAICT, according to dip1000 this code should not allow the result of `foo(x)` to be assigned to p, as it has a longer lifetime. Likewise, it should not allow foo to return &x without the parameter being annotated with `return`.
>>
>> It does all those things when you replace the body of foo with just `return &x;`.
>
> Not really. When you do that, DMD rejects the `return` statement, not the assignment `p = foo(x);`.

But that is what you want right? You want it to error out.

My reasoning is as follows, because dataflow analyses is missing, you need to simplify the body of foo to `return &x`. Then the compilers errors out saying you are escaping a reference and maybe you need to add the return annotation. After you fix that the compiler errors out saying "Error: address of variable x assigned to p with longer lifetime". Which is exactly what we want.

Ergo, missing dataflow analyses.

> It works as expected when you mark the parameter with `return`. This has nothing to do with missing flow analysis. It's just a bug.

I agree that it will be better to get the 2 errors the first time, but is that a bug?

> ----
> @safe:
>
> int* fp(int* p) { return p; } /* This works. */
> int* fr(ref int r)  { return &r; } /* So this should work too. */
>
> void main()
> {
>     int x;
>     fp(&x); /* This doesn't work. */
>     fr(x); /* So this shouldn't work either. */
> }
> ----

1) fp isn't `scope int*` so the function body compiles without errors;
2) you cannot return a reference to a local, so the function `fr` errors out correctly;
3) you cannot pass a reference to a local to a non-scope argument, so fp(&x) errors out correctly;
4) it is perfectly valid to pass a value to a ref parameter;

Still, I agree that it will be better to get the 2 errors the first time.

On 24.09.19 10:05, Sebastiaan Koppe wrote:
> On Monday, 23 September 2019 at 20:50:30 UTC, ag0aep6g wrote:
[...]
>> Not really. When you do that, DMD rejects the `return` statement, not the assignment `p = foo(x);`.
> 
> But that is what you want right? You want it to error out.

Not at that point. Unless `ref` implies restrictions that pointers don't have, `foo` should compile.

> My reasoning is as follows, because dataflow analyses is missing, you need to simplify the body of foo to `return &x`. Then the compilers errors out saying you are escaping a reference and maybe you need to add the return annotation. After you fix that the compiler errors out saying "Error: address of variable x assigned to p with longer lifetime". Which is exactly what we want.
> 
> Ergo, missing dataflow analyses.

Aside: Your reasoning is circular. You start and end with "dataflow analysis is missing".

When using a pointer instead of `ref`, the code is rejected as expected even with the more complex body:

----
@safe:

int* foo(int* x)
{
    int* a = x;
    return x;
}

void main() {
    int* p;
    {
        int x;
        p = foo(&x); /* error here */
    }
}
----

So it works just fine with pointers. But maybe `ref`s are intentionally treated differently from pointers. And maybe that means that they need more advanced dataflow analysis which isn't currently implemented. If that's so, it's a hole in -dip1000.

So I'm saying: Just treat `ref`s like pointers and the trouble disappears. I suspect that it's actually intended to work like that, and we're just looking at a good old bug.

>> It works as expected when you mark the parameter with `return`. This has nothing to do with missing flow analysis. It's just a bug.
> 
> I agree that it will be better to get the 2 errors the first time, but is that a bug?

Memory corruption in @safe code is always a bug.

[...]
> 1) fp isn't `scope int*` so the function body compiles without errors;

Yup.

> 2) you cannot return a reference to a local, so the function `fr` errors out correctly;

A `ref` parameter is significantly different from a true local. In the context of DIP 1000, it's more straight-forward to treat it like a pointer.

> 3) you cannot pass a reference to a local to a non-scope argument, so fp(&x) errors out correctly;

Yup.

> 4) it is perfectly valid to pass a value to a ref parameter;

Not when we treat `ref` like the pointer that it really is.

Pretending that `ref` is anything but a pointer wearing a fancy hat just leads to headaches like the one we're experiencing right now.