Hello everyone,

I am trying to fix a regression with regards to -dip1000 [1], but I am terribly confused on what the behavior should be. Example:

class MinPointerRecorder
{
    int* minPrice;
    void update(ref int price) @safe
    {
        minPrice = &price; /* Should not compile. */
    }
}

Compile that code without -dip1000 and you get an error: " Error: cannot take address of local `a` in `@safe` function `test`". Compile with dip1000 and the error goes away. Is DIP1000 supposed to relax conditions for non-scoped pointers/references? I would assume that dip1000 should impose harder restrictions, not relax them. Normally, in @safe code you are not allowed to take the address of a local or a parameter, however, it seems that with -dip1000 that is allowed and the compiler tries to infer `scope`.

What happens in this specific case is that price is inferred to be non-scope and therefore is allowed to be passed to `minPrice` leading to memory coruption (see the bug report).

Does anyone know what exactly is the intended behavior? Unfortunately both the spec and the DIP [2] do not explicitly mention this cases.

Cheers,
RazvanN


[1] https://issues.dlang.org/show_bug.cgi?id=21212
[2] https://github.com/dlang/DIPs/blob/master/DIPs/other/DIP1000.md

On Thursday, 18 February 2021 at 10:05:52 UTC, RazvanN wrote:

> Normally, in @safe code you are not allowed to take the address of a local or a parameter, however, it seems that with -dip1000 that is allowed and the compiler tries to infer `scope`.

Isn't that the whole point or at least a big part of DIP100 (or was that @live)? It can track the pointers and knows that it's safe if the pointer doesn't live longer than the local variable.

--
/Jacob Carlborg

On Thursday, 18 February 2021 at 11:30:03 UTC, Jacob Carlborg wrote:
> On Thursday, 18 February 2021 at 10:05:52 UTC, RazvanN wrote:
>
>> Normally, in @safe code you are not allowed to take the address of a local or a parameter, however, it seems that with -dip1000 that is allowed and the compiler tries to infer `scope`.
>
> Isn't that the whole point or at least a big part of DIP100 (or was that @live)? It can track the pointers and knows that it's safe if the pointer doesn't live longer than the local variable.
>
> --
> /Jacob Carlborg

The question is what is the difference between scope and non-scope pointers? I would assume that if you don't use scope at all, then the code should have the same behavior as it would if you did not use -dip1000.

On Thursday, 18 February 2021 at 10:05:52 UTC, RazvanN wrote:
> Hello everyone,
>
> I am trying to fix a regression with regards to -dip1000 [1], but I am terribly confused on what the behavior should be. Example:
>
> class MinPointerRecorder
> {
>     int* minPrice;
>     void update(ref int price) @safe
>     {
>         minPrice = &price; /* Should not compile. */
>     }
> }
>
> Compile that code without -dip1000 and you get an error: " Error: cannot take address of local `a` in `@safe` function `test`". Compile with dip1000 and the error goes away. Is DIP1000 supposed to relax conditions for non-scoped pointers/references? I would assume that dip1000 should impose harder restrictions, not relax them. Normally, in @safe code you are not allowed to take the address of a local or a parameter, however, it seems that with -dip1000 that is allowed and the compiler tries to infer `scope`.
>
> What happens in this specific case is that price is inferred to be non-scope and therefore is allowed to be passed to `minPrice` leading to memory coruption (see the bug report).
>
> Does anyone know what exactly is the intended behavior? Unfortunately both the spec and the DIP [2] do not explicitly mention this cases.
>
> Cheers,
> RazvanN
>
>
> [1] https://issues.dlang.org/show_bug.cgi?id=21212
> [2] https://github.com/dlang/DIPs/blob/master/DIPs/other/DIP1000.md

The person that you should be asking this question towards is walter himself. He is the one who is driving force behind the implementation of the dip. We have to stop some of his PR request regarding dip1000 without spec documentation as it going to create some confusion in the future.

-Alex

On Thursday, 18 February 2021 at 10:05:52 UTC, RazvanN wrote::
>
> class MinPointerRecorder
> {
>     int* minPrice;
>     void update(ref int price) @safe
>     {
>         minPrice = &price; /* Should not compile. */
>     }
> }
> [snip]
> Does anyone know what exactly is the intended behavior? Unfortunately both the spec and the DIP [2] do not explicitly mention this cases.
>

That member function itself can compile, no problem. But -dip1000 should prevent calling it with a local argument:
```
@safe void foo(int arg)
{ auto mpr= new MinPointRecorder();
  mpr.update(*new int); //okay
  mpr.update(arg); //won't do - escaping reference to local variable
}
```

Or that's my understanding at least.

On Thursday, 18 February 2021 at 14:33:19 UTC, Dukc wrote:
> On Thursday, 18 February 2021 at 10:05:52 UTC, RazvanN wrote::
>>
>> class MinPointerRecorder
>> {
>>     int* minPrice;
>>     void update(ref int price) @safe
>>     {
>>         minPrice = &price; /* Should not compile. */
>>     }
>> }
>> [snip]
>> Does anyone know what exactly is the intended behavior? Unfortunately both the spec and the DIP [2] do not explicitly mention this cases.
>>
>
> That member function itself can compile, no problem. But -dip1000 should prevent calling it with a local argument:

This was my understanding as well, based on paragraph 4 of the spec's section on "Return Ref Parameters" [1]:

> If the function returns void, and the first parameter is ref or out, then all subsequent return ref parameters are considered as being assigned to the first parameter for lifetime checking. The this reference parameter to a struct non-static member function is considered the first parameter.

(I assume "struct" here is supposed to be "struct or class".)

But if you try it with a struct instead of a class, the compiler *does* flag the assignment as invalid, even with an explicit `return` annotation:

struct MinPointerRecorder
{
    int* minPrice;
    void update(return ref int price) @safe
    {
        minPrice = &price;
        // Error: address of variable `price` assigned to `this` with longer lifetime
    }
}

On the other hand, if you change the `ref` to a `scope` pointer, it compiles again:

struct MinPointerRecorder
{
    int* minPrice;
    void update(return scope int* price) @safe
    {
        minPrice = price; // Ok
    }
}

So, I'm not sure what the correct answer here is.

[1] https://dlang.org/spec/function.html#return-ref-parameters

On 2/18/2021 2:05 AM, RazvanN wrote:
> I am terribly confused on what the behavior should be. Example:
> 
> class MinPointerRecorder
> {
>      int* minPrice;
>      void update(ref int price) @safe
>      {
>          minPrice = &price; /* Should not compile. */
>      }
> }

To figure it out, rewrite it in the form of using pointers:

    void update(int** p, ref int price) @safe
    {
        *p = &price;
    }

which shouldn't compile, but does.

On Friday, 19 February 2021 at 00:48:59 UTC, Walter Bright wrote:
> To figure it out, rewrite it in the form of using pointers:
>
>     void update(int** p, ref int price) @safe
>     {
>         *p = &price;
>     }
>

Wouldn't the pointer rewrite be
```
void update(int** p, int* price) @safe
{ *p = price;
}
```
?

The compiler should infer that `p` can be `scope`, but `price` can't.

> which shouldn't compile

Why?

On 2/19/2021 1:49 AM, Dukc wrote:
> Wouldn't the pointer rewrite be
> ```
> void update(int** p, int* price) @safe
> { *p = price;
> }
> ```
> ?

ref and * are handled differently.


> The compiler should infer that `p` can be `scope`, but `price` can't.
> 
>> which shouldn't compile
> 
> Why?

Because the address of price escapes the function, and that's not allowed for ref variables.

On Friday, 19 February 2021 at 10:52:23 UTC, Walter Bright wrote:
>
> Because the address of price escapes the function, and that's not allowed for ref variables.

So `ref` is always `scope`? Okay, got it.