July 17, 2019
On 7/17/2019 11:59 AM, Olivier FAURE wrote:
> The current example isn't good enough. It's not @safe (it calls free() directly), it's not RAII (the structure doesn't manage its own memory),

That's not the point. A container with an @safe interface can do this, for example, simply by appending to the container contents the pointer to the old contents can be invalidated. The point of the example is to illustrate the problem clearly, not obfuscate it with the complexities of a container object.

More generally, any time there are two mutable pointers to the same memory object, one can invalidate the other. This is the principle pointed out in the "Prior Work" section. The DIP is not intended to be a tutorial on this, which is why there's a link to more explanation. It has nothing to do with RAII, and thinking about it in terms of RAII will miss the generality of the concept.


> and it doesn't even show what a syntax error would look like.

A language specification does not specify the text of error messages, merely that a particular configuration is not legal.


> The proposal says inter-statement checking isn't done, but it doesn't explain what is or isn't inter-statement checking. Do nested expressions count?

The current language spec makes it clear what a statement is and what an expression is.
July 18, 2019
On Wednesday, 17 July 2019 at 23:26:20 UTC, Walter Bright wrote:
> More generally, any time there are two mutable pointers to the same memory object, one can invalidate the other. This is the principle pointed out in the "Prior Work" section. The DIP is not intended to be a tutorial on this, which is why there's a link to more explanation.

By "more explanation", do you mean the Rust tutorial? Because (1) pointing at an entire flippin' language and saying "There, that's prior work, just read it if you have any questions" seems pretty lazy to me, (2), this DIP obviously isn't trying to implement everything single Rust construct supporting borrowing (eg lifetimes, single initialization, and yes, interstatement checking), and D isn't a language made from the ground up like Rust, which means its implementation is going to be different in ways that need to be documented.

Like, is D going to implement the "one assignment, one move" rule from Rust? Lifetime parameters to structs?

> It has nothing to do with RAII, and thinking about it in terms of RAII will miss the generality of the concept.

Rust does have RAII, though. In fact, containers like Vec<T> Box<T>, and Rc<T> rely explicitly on RAII, and can't work with lifetimes alone.

> A language specification does not specify the text of error messages, merely that a particular configuration is not legal.

Yeah, but your examples don't even show any illegal configuration.

The DIP says "The checks would only be enforced for @safe code.", but your examples only show @system code.

> The current language spec makes it clear what a statement is and what an expression is.

So:

    foo(identity(&s), s.get()); // Does this compile in @safe ?

> That's not the point. A container with an @safe interface can do this, for example, simply by appending to the container contents the pointer to the old contents can be invalidated.

Can you show an example of this? If you don't want to add to the DIP, then in this thread?

Or even tell me whether the example code I already posted matches what you had in mind?

Right now, containers can already allocate memory to append new contents, so the DIP wouldn't improve that aspect. What they can't do is call free() on the old contents, except free() can't be called in safe code, which imply containers would need a @trusted method to free memory, which comes with problems the proposal doesn't address.
July 18, 2019
On 15.07.19 17:23, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1021, "Argument Ownership and Function Calls":
> 
> https://github.com/dlang/DIPs/blob/793f83911fdc8c88c6ef34e6a36b5e11e3e574e5/DIPs/DIP1021.md 

The DIP isn't clear in what it is supposed to achieve. Is there actual @safe code that can cause memory corruption currently, which the DIP would prevent? Or would the DIP allow code to be @trusted that can't be currently?

As far as I understand, the DIP would allow `free` to be @trusted in certain situations.

I.e., this isn't okay currently:

    struct S
    {
        byte* ptr;
        byte* get() { return ptr; }
        this(byte value) @trusted
        {
            ptr = cast(byte*) malloc(1);
            *ptr = value;
        }
        void clear() @trusted
        {
            free(ptr);
            ptr = null;
        }
    }

... because a function could call `clear` while already having obtained `ptr`.

But it would be with the DIP, maybe? Is that the point of the DIP?

Except it still wouldn't be 100% ok, because @safe code could set `ptr = new byte;` and then `free` would be called on GC memory.
July 18, 2019
On Thursday, 18 July 2019 at 09:09:37 UTC, ag0aep6g wrote:
> ... because a function could call `clear` while already having obtained `ptr`.

I think the DIP allows a destructor freeing ptr to be trusted:

import core.stdc.stdlib, std.stdio;

struct S
{
    private byte* ptr;
    import core.stdc.stdlib;

    // FIXME return scope doesn't seem to work (with -dip1000)
    byte* get() @safe return scope { return ptr; }
    this(byte value) @trusted
    {
        ptr = cast(byte*) calloc(1, 1);
        *ptr = value;
    }
    ~this() @trusted
    {
        free(ptr);
    }
}

@safe:

// won't compile with DIP 1021
void bad(ref S s, ref byte b)
{
    s.destroy;
    b++;
}

byte* f()
{
    auto s = S(5);
    bad(s, *s.get);
    return s.get; // this shouldn't compile, but it does
}

> Except it still wouldn't be 100% ok, because @safe code could set `ptr = new byte;` and then `free` would be called on GC memory.

It is @safe outside the module that S is defined in with a private ptr (modulo .tupleof).
July 18, 2019
On Thursday, 18 July 2019 at 09:09:37 UTC, ag0aep6g wrote:
> On 15.07.19 17:23, Mike Parker wrote:
>> This is the feedback thread for the first round of Community Review for DIP 1021, "Argument Ownership and Function Calls":
>> 
>> https://github.com/dlang/DIPs/blob/793f83911fdc8c88c6ef34e6a36b5e11e3e574e5/DIPs/DIP1021.md
>
> The DIP isn't clear in what it is supposed to achieve. Is there actual @safe code that can cause memory corruption currently, which the DIP would prevent? Or would the DIP allow code to be @trusted that can't be currently?
>
> As far as I understand, the DIP would allow `free` to be @trusted in certain situations.
>
> I.e., this isn't okay currently:
>
>     struct S
>     {
>         byte* ptr;
>         byte* get() { return ptr; }
>         this(byte value) @trusted
>         {
>             ptr = cast(byte*) malloc(1);
>             *ptr = value;
>         }
>         void clear() @trusted
>         {
>             free(ptr);
>             ptr = null;
>         }
>     }
>
> ... because a function could call `clear` while already having obtained `ptr`.
>
> But it would be with the DIP, maybe? Is that the point of the DIP?
>
> Except it still wouldn't be 100% ok, because @safe code could set `ptr = new byte;` and then `free` would be called on GC memory.

There seems to be a lot of unclearness about this dip which a few failing and success cases would fix. I'm not sure why not just add a few examples to make it more clear...

Anyway, I think the dip has nothing to with any case other than the same variable appearing in the same statement. That is the extent of this DIPs scope, but it's quite unclear from the DIP. I think it basically asks:

"is the same variable seen in a single statement as an argument to more than one ref parameter?" if they are constant then ok, if not then error.

so:

f(v, v.ptr) ; // the check applies

a = v.ptr
f(v, a); // the check doesn't apply
July 18, 2019
On Thursday, 18 July 2019 at 10:42:15 UTC, Nick Treleaven wrote:
> // won't compile with DIP 1021
> void bad(ref S s, ref byte b)

Or rather, this line won't compile:

>     bad(s, *s.get);
July 18, 2019
On 18.07.19 12:42, Nick Treleaven wrote:
> On Thursday, 18 July 2019 at 09:09:37 UTC, ag0aep6g wrote:
[...]
> struct S
> {
>      private byte* ptr;
>      import core.stdc.stdlib;
[...]
>      ~this() @trusted
>      {
>          free(ptr);
>      }
> }
> 
> @safe:
> 
> // won't compile with DIP 1021
> void bad(ref S s, ref byte b)
> {
>      s.destroy;
>      b++;
> }
[...]

I'm not sure if there's a meaningful difference between your code and mine. You're calling the destructor explicitly with `destroy`, So it's practically the same as my `clear` method, no?

Anyway, I think we're on the same page: The goal of DIP 1021 seems to be to allow marking calls to `free` (and similar functions) as @trusted in certain situations. The DIP should say this, and give an example.

>> Except it still wouldn't be 100% ok, because @safe code could set `ptr = new byte;` and then `free` would be called on GC memory.
> 
> It is @safe outside the module that S is defined in with a private ptr (modulo .tupleof).

That's a common hack, but strictly speaking it's an invalid use of @trusted. An @trusted function must be safe regardless of where it's called from. I.e., it must also be safe when called from within the same module.

But finding a solution to that problem is probably outside of the scope of the DIP being discussed.
July 18, 2019
On Thursday, 18 July 2019 at 08:33:22 UTC, Olivier FAURE wrote:
> On Wednesday, 17 July 2019 at 23:26:20 UTC, Walter Bright wrote:
>> This is the principle pointed out in the "Prior Work" section. The DIP is not intended to be a tutorial on this, which is why there's a link to more explanation.
>
> By "more explanation", do you mean the Rust tutorial? Because (1) pointing at an entire flippin' language and saying "There, that's prior work, just read it if you have any questions"

The prior work section does not point to top-level rust-lang.org as you seem to suggest, but to this specific link:
https://doc.rust-lang.org/1.8.0/book/references-and-borrowing.html#the-rules

> The DIP says "The checks would only be enforced for @safe code.", but your examples only show @system code.

You're right. I've made an example here:
https://forum.dlang.org/post/gdptdtqcethrldpsicss@forum.dlang.org

I see my example is wrong, the destructor can't be @trusted, but the DIP can at least detect a memory-safety problem.

>     foo(identity(&s), s.get()); // Does this compile in @safe ?

Since DIP 25, the compiler knows the result of identity has the same scope as its argument (in @safe code), so I expect the above would not compile with this DIP.

July 18, 2019
On Thursday, 18 July 2019 at 11:16:02 UTC, ag0aep6g wrote:
> I'm not sure if there's a meaningful difference between your code and mine. You're calling the destructor explicitly with `destroy`, So it's practically the same as my `clear` method, no?

I think the difference is that destroy takes an S by ref, so the compiler could raise an error about p potentially outliving s:

void maybeBad(ref S s); // might call s.destroy
scope p = s.get;
s.maybeBad;
// p may now be invalid

But I don't think this is proposed by this DIP or supported by DIP 1000.

> Anyway, I think we're on the same page: The goal of DIP 1021 seems to be to allow marking calls to `free` (and similar functions) as @trusted in certain situations. The DIP should say this, and give an example.

I realized my example shouldn't mark the destructor as @trusted, because of the DIP limitation section:
https://github.com/dlang/DIPs/blob/793f83911fdc8c88c6ef34e6a36b5e11e3e574e5/DIPs/DIP1021.md#limitations

> That's a common hack, but strictly speaking it's an invalid use of @trusted. An @trusted function must be safe regardless of where it's called from. I.e., it must also be safe when called from within the same module.

So we need C++ private then.

> But finding a solution to that problem is probably outside of the scope of the DIP being discussed.

Yes.

July 18, 2019
On Thursday, 18 July 2019 at 11:18:29 UTC, Nick Treleaven wrote:
> The prior work section does not point to top-level rust-lang.org as you seem to suggest, but to this specific link:
> https://doc.rust-lang.org/1.8.0/book/references-and-borrowing.html#the-rules

Saying "an entire language" was hyperbolic and maybe a little unfair, but I maintain that this is way too broad.

Rust as it exists today is a vast language designed almost from the ground up to accommodate the borrowing model. It relies on several features that do not (and may never) exist in D, so saying "we're going to use a memory model like the one Rust has" is extremely vague, and doesn't explain how various corner cases will be handled.

This is especially true for this DIP, which explicitly doesn't include function-wide data flow analysis like Rust does.