So, there is a pull request (https://github.com/dlang/dmd/pull/5972)
which is intended to implement DIP1000
(https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md).

Problem is that Walter wants to prioritize moving on with it (which is reasonable) but the PR is too complicated to be reviewed by most other developers.

Trying to solve it, I have been experimenting lately with various acceptance tests trying to come up with useful design snippets enabled by that PR. Rationale is that if actual semantics are clear and examples are all validated as included test cases, review of the code itself becomes less important.

I'd like to have more open feedback about it because in its current shape PR5972 doesn't seem useful enough to me to be merged at all - and much less powerful than DIP1000 seemed to promise.

I'll start separate sub-threads with more details.

On 10/16/2016 04:45 PM, Dicebot wrote:
> I'll start separate sub-threads with more details.

This snippet is a variation of RCSlice example from the DIP:

------------------------------------------------------------
@safe unittest
{
    @safe static struct RCPointer (T)
    {
        private T*      payload;
        private size_t* count;

        this (T initializer)
        {
            this.payload  = new T;
            *this.payload = initializer;
            this.count    = new size_t;
            *count = 1;
        }

        this (this)
        {
            if (this.count)
                ++(*this.count);
        }

        @trusted ~this ()
        {
            if (this.count && !--(*this.count))
            {
                delete payload;
                delete count;
            }
        }

	// must bind lifetime of return value to lifetime of `this`
        T* get () return scope
        {
            return this.payload;
        }

        alias get this;
    }

    auto ptr = RCPointer!int(42);
    auto leak2 = ptr.get(); // must not compile unless spec says 'auto'
                            // can deduce scope
    int* leak1 = ptr.get(); // must not compile

    void foo (RCPointer!int)
    {
        assert(*(ptr.count) == 2);
    }

    foo(ptr);

    void bar (ref int)
    {
        assert(*(ptr.count) == 1);
    }

    bar(*ptr);
}

------------------------------------------------------------

DIP1000 says that `return scope` applied to a method means applying it to aggregate `this`. It also says that struct is viewed as a composition of all its fields when it comes to lifetime.

Natural consequence is that it should be possible to define `get` method like in this example to ensure that pointer won't outlive struct instance it came from.

This snippet currently compiles, failing to reject both "leak" lines.

On 10/16/2016 04:45 PM, Dicebot wrote:
> I'll start separate sub-threads with more details.

This is probably most ambitious snippet I have come up with so far. It seems to be possible within current DIP1000 definition but doesn't work for other reasons I wanted to discuss:

------------------------------------------------------------
@safe unittest
{
    @safe static struct Owner
    {
        int data;

        // must bind lifetime of return pointer/ref to one of `this`
	// Error: function has 'return' but does not return any
 	// indirections
        auto borrow () return scope
        {
            struct Range
            {
                int* data;
                enum empty = false;
                auto front () { return *this.data; }
                void popFront() { (*this.data)++; }
            }

            return Range(&this.data); // Error: cannot take address of
                                      // parameter this in @safe
function borrow
        }
    }

    Owner instance;
    // must work, lifetime of 'instance' exceeds one of x :
    scope x = instance.borrow();
    // must not work, infinite lifetime
    auto y  = instance.borrow();
}
------------------------------------------------------------

First error simply to be simply a bug, because it contradicts "struct is a composition of its fields" statement from DIP1000.

Second error is an existing limitation of @safe code (can't take pointer from ref) which seems extremely limiting now that tools to enforce scope lifetime come into play.

On 10/16/2016 7:02 AM, Dicebot wrote:
>     Owner instance;
>     // must work, lifetime of 'instance' exceeds one of x :
>     scope x = instance.borrow();
>     // must not work, infinite lifetime
>     auto y  = instance.borrow();
> }

> Second error is an existing limitation of @safe code (can't take pointer
> from ref) which seems extremely limiting now that tools to enforce scope
> lifetime come into play.

It works because 'scope' is inferred as a storage class when an expression that needs scope is used to initialize a local. y gets a subset of the life of instance. Doing such inference makes using -transition=safe far more palatable.

BTW, reducing the examples to their minimums helps a lot.

On 10/16/2016 7:02 AM, Dicebot wrote:
> 	// [1] Error: function has 'return' but does not return any
>  	// indirections
>         auto borrow () return scope
>         {
>             struct Range
>             {
>                 int* data;
>             }
>
>             return Range(&this.data); // [2] Error: cannot take address of
>                                       // parameter this in @safe
> function borrow
>         }
>
> First error [1] simply to be simply a bug, because it contradicts "struct is
> a composition of its fields" statement from DIP1000.

I'd ignore that error because it is a cascaded error.

(It is helpful to label errors so it's easier to see which you're referring to. I added such annotations to the quoted part.)

As for error [2], 'ref' is meant to be a restricted pointer. Converting a restricted pointer to a regular pointer, which is what &this does, undermines the whole point of having 'ref', and so is disallowed in @safe code.

Consider:

  @safe int* foo(return ref int r) {
    return &r; // Error: cannot take address of parameter r in @safe function foo
  }

Making it @trusted works. But even if @trusted, escape checking is still done, even in @system code:

  @trusted int* foo(return ref int r) {
    return &r; // no error, 'cuz it's trusted
  }

  int* bar() {
    int r;
    return foo(r); // Error: escaping reference to local variable r
  }

On 10/16/2016 6:55 AM, Dicebot wrote:
> This snippet currently compiles, failing to reject both "leak" lines.

It compiles because 'scope' is inferred for both leak1 and leak2.

On 10/16/2016 10:30 PM, Walter Bright wrote:
> On 10/16/2016 7:02 AM, Dicebot wrote:
>>     Owner instance;
>>     // must work, lifetime of 'instance' exceeds one of x :
>>     scope x = instance.borrow();
>>     // must not work, infinite lifetime
>>     auto y  = instance.borrow();
>> }

> It works because 'scope' is inferred as a storage class when an expression that needs scope is used to initialize a local. y gets a subset of the life of instance. Doing such inference makes using -transition=safe far more palatable.

Great, thanks for explanation, this will need to be mentioned clearly with example in DIP document (I'll make a PR once other uncertainties are cleared too).

However, it also keeps compiling if I change it to use explicit non-scope type:

// move Range definition outside and mark `borrow` @trusted
Range y = instance.borrow(); // compiles, but must require `scope Range y`

> BTW, reducing the examples to their minimums helps a lot.

This helps in debugging but doesn't help in defining good acceptance tests. I am looking for the latter and that means "minimal snippet that looks like useful code", not "minimal reduced test case" - and this is exactly what is needed as part of scope PR test cases to evaluate it.

Now, I would be glad to provide minimal test cases for ease of debugging too, but that is not possible until I can reason which behavior is bug and which one is intentional. Getting your comments about why certain things don't work helps to understand.

On 10/16/2016 10:53 PM, Walter Bright wrote:
> On 10/16/2016 6:55 AM, Dicebot wrote:
>> This snippet currently compiles, failing to reject both "leak" lines.
> 
> It compiles because 'scope' is inferred for both leak1 and leak2.

Where does it come from? I don't see any mention of such deduction in https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md and it doesn't look like a good idea at all.

On 10/16/2016 1:03 PM, Dicebot wrote:
> Where does it come from?

It comes from trying to compile code with the new safety checks, and having it fail to compile and require adding annotations all over the place. The idea is to infer such annotations in the obvious places. 'return' was already successfully inferred in many places, this just extends an existing practice.


> I don't see any mention of such deduction in
> https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md

It's one of those things that becomes necessary once trying to implement it.


> and it doesn't look like a good idea at all.

Inferring 'scope' and 'return' as much as possible makes @safe much more palatable. All I can say is try to break it!

On 10/16/2016 1:00 PM, Dicebot wrote:
> Range y = instance.borrow(); // compiles, but must require `scope Range y`

'scope' is inferred for 'y'.


> Now, I would be glad to provide minimal test cases for ease of debugging
> too, but that is not possible until I can reason which behavior is bug
> and which one is intentional. Getting your comments about why certain
> things don't work helps to understand.

When saying "this should compile" or "this should not compile" a minimized example is best when reasoning about it. Once we have understanding on what should and should not compile, a more meta example exploiting this would then be useful.