Issue 22528 - [dip1000] scope inference turns return-ref into return-scope
Summary: [dip1000] scope inference turns return-ref into return-scope
Status: RESOLVED DUPLICATE of issue 20881
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 blocker
Assignee: No Owner
URL:
Keywords: accepts-invalid
Depends on:
Blocks:
 
Reported: 2021-11-20 08:00 UTC by thomas.bockman
Modified: 2021-11-27 20:20 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description thomas.bockman 2021-11-20 08:00:04 UTC
In the program below, UniqueD.borrowA and uniqueD.borrowB differ only in that borrowA is a template.

When compiled with dip1000 enabled, the return annotation works correctly for the non-template borrowB, but does not work for the template borrowA.

//////////////////////////////////////////////
module app;

import core.stdc.stdlib : malloc, free;
import core.lifetime : emplace;

import std.traits;

class D { }

struct UniqueD {
    private D _target;
    inout(D) borrowA()() return inout pure @safe nothrow @nogc {
        return _target; }
    inout(D) borrowB() return inout pure @safe nothrow @nogc {
        return _target; }

    this(this This)(const(bool) value) @trusted {
        if(value) {
            _target = cast(typeof(_target)) malloc(__traits(classInstanceSize, D));
            emplace(_target);
        } else
            _target = null;
    }

    @disable this(this);
    @disable this(ref typeof(this));
    @disable ref typeof(this) opAssign(ref typeof(this));

    ~this() @trusted {
        if(_target !is null) {
            destroy!false(_target);
        	free(cast(void*) _target);
            _target = null;
        }
    }
}

void main() @safe {
    static D staticD = null;

    UniqueD uniqueD = true;
    staticD = uniqueD.borrowA; // Accepts invalid only if borrow is a template.
    staticD = uniqueD.borrowB; // Error: address of variable `uniqueD` assigned to `staticD` with longer lifetime
}
//////////////////////////////////////////////

This bug is a blocker to achieving @safe C++ or Rust style memory management in D for some very basic use cases, along with:

https://issues.dlang.org/show_bug.cgi?id=21981#add_comment
Comment 1 Dennis 2021-11-24 14:33:09 UTC
What's happening is that `borrowA` gets `scope` inferred because it's a template. 
You can add this line to borrowA to break scope inference and make it behave like borrowB:
```
inout D temp = this._target;
```

Here's a more reduced test case:
```
@safe:

struct S {
    int* ptr;
    
    auto borrowA() return {
        return ptr;
    }
    
    int* borrowB() return /*scope inferred*/ {
        return ptr;
    }
}

void main() {
    static int* global;
    S s;
    global = s.borrowA;
    global = s.borrowB;
}
```
Comment 2 Dennis 2021-11-24 15:00:06 UTC

*** This issue has been marked as a duplicate of issue 20881 ***
Comment 3 thomas.bockman 2021-11-26 02:01:48 UTC
(In reply to Dennis from comment #1)
> What's happening is that `borrowA` gets `scope` inferred because it's a
> template.

I don't get it. `return` restricts how the return value may be used by the caller, and `scope` restricts how the `this` reference may be used by the callee.

Why does an inferred restriction in once place effectively remove my explicit restriction from another?

> You can add this line to borrowA to break scope inference and make it behave
> like borrowB:
> ```
> inout D temp = this._target;
> ```

Thanks for the workaround; I'll try that out.

It does look fragile though, as it's basically tricking the compiler. At any time the compiler could be upgraded to be smart enough to recognize that this line is, in context, a no-op and can be ignored for analysis purposes.
Comment 4 Dennis 2021-11-26 09:48:40 UTC
(In reply to thomas.bockman from comment #3)
> Why does an inferred restriction in once place effectively remove my
> explicit restriction from another?

Because the implementation is bad, it currently has flags STC.scope_ and STC.return_ and infers the meaning of STC.return from context. Walter is working on using STC.returnScope instead, see issue 22541 and https://github.com/dlang/dmd/pull/13357

> At any
> time the compiler could be upgraded to be smart enough to recognize that
> this line is, in context, a no-op and can be ignored for analysis purposes.

Yes, it relies on issue 20674. I do think this issue will be solved before that, but you could also add a unittest with `assert(!__traits(compiles, () @safe {...}));` to test that escaping a reference isn't allowed, so you'll be aware when the workaround breaks.