www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DIP 1021--Argument Ownership and Function Calls--Community Review

reply Mike Parker <aldacron gmail.com> writes:
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

All review-related feedback on and discussion of the DIP should 
occur in this thread. The review period will end at 11:59 PM ET 
on July 30, or when I make a post declaring it complete.

At the end of Round 2, if further review is deemed necessary, the 
DIP will be scheduled for another round of Community Review. 
Otherwise, it will be queued for the Final Review and Formal 
Assessment.

For some additional context on this DIP, please see Walter's 
latest post on the D Blog titled "Ownership and Borrowing in D":

https://dlang.org/blog/2019/07/15/ownership-and-borrowing-in-d/

But *please do not discuss the blog post or its topic in this 
thread*! Use my post in the Announce forum for that:

https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org

Anyone intending to post feedback in this thread is expected to 
be familiar with the reviewer guidelines:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

I will be moving (copying, deleting, and pasting in a new thread) 
posts that do not adhere to the reviewer guidelines. So please 
keep all comments in this thread focused on DIP 1021.

Thanks in advance to all who participate.
Jul 15 2019
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, 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


Do these new checks also handle the case where one of the 
references is passed implicitly to a delegate or nested function 
via their context? In other words, if we re-write the example 
from the DIP to use a nested function, like so:

struct S {
     byte* ptr;
     ref byte get() { return *ptr; }
}

void test() {
     S s;
     s.ptr = cast(byte*) malloc(1);

     void foo(ref byte b) {
         free(s.ptr);
         b = 4;
     }

     foo(s.ptr);
}

Will the call to `foo` here be correctly flagged as taking 
multiple mutable references to `s`?
Jul 15 2019
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
But *please do not discuss the blog post or its topic in this thread*! Use the 
post in the Announce forum for that:

https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org
Jul 15 2019
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/15/2019 8:55 AM, Paul Backus wrote:

 On Monday, 15 July 2019 at 15:23:32 UTC, 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/793f83911fdc8c88c6ef34e6a36b5e11e3e5
4e5/DIPs/DIP1021.md 


 
 Do these new checks also handle the case where one of the references is passed 
 implicitly to a delegate or nested function via their context?


Yes. Access to such variables is treated "as if" they were passed by reference 
as arguments to the function. Of course, in reality they actually are passed by
ref!



 In other words, 
 if we re-write the example from the DIP to use a nested function, like so:
 
 struct S {
      byte* ptr;
      ref byte get() { return *ptr; }
 }
 
 void test() {
      S s;
      s.ptr = cast(byte*) malloc(1);
 
      void foo(ref byte b) {
          free(s.ptr);
          b = 4;
      }
 
      foo(s.ptr);
 }
 
 Will the call to `foo` here be correctly flagged as taking multiple mutable 
 references to `s`?


Yes, the parameter list will be treated as:

     void foo(ref byte b, ref S s);

and the call:

     foo(s.ptr, s);
Aug 02 2019
prev sibling next sibling parent reply Max Haughton <maxhaton gmail.com> writes:
I think this DIP should at least consider expanding its scope to 
use the reaching definition analysis.

It seems slightly pointless if, even if it's already opt in, that 
we can bypass it so easily e.g. by assigning a temporary.

Or is it really *that* complicated to allow the compiler to 
analyse this?
Jul 15 2019
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
But *please do not discuss the blog post or its topic in this thread*! Use the 
post in the Announce forum for that:

https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org
Jul 15 2019
next sibling parent reply Brad Roberts <braddr puremagic.com> writes:
On 7/15/2019 6:55 PM, Walter Bright via Digitalmars-d wrote:

 But *please do not discuss the blog post or its topic in this thread*! 
 Use the post in the Announce forum for that:
 
 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


Wait, what?  Why are we encouraging / having _discussions_ on the 
_announce_ list?  I know we've never prevented them in the past, but 
this is probably the first time I've seen active encouragement.  While 
I'm not an particularly active member of the community these days, I 
strongly disagree with this policy.
Jul 15 2019
next sibling parent SashaGreat <sasha gmail.com> writes:
On Tuesday, 16 July 2019 at 03:04:53 UTC, Brad Roberts wrote:

 On 7/15/2019 6:55 PM, Walter Bright via Digitalmars-d wrote:

 But *please do not discuss the blog post or its topic in this 
 thread*! Use the post in the Announce forum for that:
 
 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


 Wait, what?  Why are we encouraging / having _discussions_ on 
 the _announce_ list?  I know we've never prevented them in the 
 past, but this is probably the first time I've seen active 
 encouragement.  While I'm not an particularly active member of 
 the community these days, I strongly disagree with this policy.


For what I understood this thread is exclusively for the DIP-1021 
not for discussing the blog post.

Sasha.
Jul 15 2019
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/15/2019 8:04 PM, Brad Roberts wrote:

 On 7/15/2019 6:55 PM, Walter Bright via Digitalmars-d wrote:

 But *please do not discuss the blog post or its topic in this thread*! Use the 
 post in the Announce forum for that:

 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


 
 Wait, what?  Why are we encouraging / having _discussions_ on the _announce_ 
 list?  I know we've never prevented them in the past, but this is probably
the 
 first time I've seen active encouragement.  While I'm not an particularly
active 
 member of the community these days, I strongly disagree with this policy.


It was mainly to avoid people having to read both threads.
Jul 15 2019
parent Mike Parker <aldacron gmail.com> writes:
On Tuesday, 16 July 2019 at 06:14:02 UTC, Walter Bright wrote:

 On 7/15/2019 8:04 PM, Brad Roberts wrote:

 On 7/15/2019 6:55 PM, Walter Bright via Digitalmars-d wrote:

 But *please do not discuss the blog post or its topic in this 
 thread*! Use the post in the Announce forum for that:

 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


 
 Wait, what?  Why are we encouraging / having _discussions_ on 
 the _announce_ list?  I know we've never prevented them in the 
 past, but this is probably the first time I've seen active 
 encouragement.  While I'm not an particularly active member of 
 the community these days, I strongly disagree with this policy.


 It was mainly to avoid people having to read both threads.


To everyone discussing this aspect of my post: It's off topic! 
Since I obviously was not clear in my intent, I'll refrain from 
cutting these posts and pasting them in a new thread.

To clarify, discussion of Walter's blog post is off-topic in this 
thread. If you want to provide feedback on the blog post, the 
place to do so is in the thread where I announced it (as is often 
the case) or in a new thread in General. Just not here.

If you want to discuss how DIP 1021 relates to the topic of the 
blog post to clarify your understanding of the DIP, this review 
thread is fine.

If you want to discuss if we should discuss things in the 
announce forum, please start a new thread!

I only ask this because it can be time-consuming to read through 
long review threads trying to pick out the relevant information 
for my review summary. And for anyone coming to the review thread 
later, they shouldn't have to wade through irrelevant discussion 
to get to the good stuff.

Thanks!

So. Back on topic.
Jul 15 2019
prev sibling parent reply Les De Ridder <les lesderid.net> writes:
On Tuesday, 16 July 2019 at 01:55:18 UTC, Walter Bright wrote:

 But *please do not discuss the blog post or its topic in this 
 thread*! Use the post in the Announce forum for that:

 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


I believe Mike meant that this thread should be used for 
discussion of
the DIP, while the other thread should be used for discussion of 
the
blog post/OB/ live.
Jul 15 2019
parent SashaGreat <sasha gmail.com> writes:
On Tuesday, 16 July 2019 at 03:19:33 UTC, Les De Ridder wrote:

 On Tuesday, 16 July 2019 at 01:55:18 UTC, Walter Bright wrote:

 But *please do not discuss the blog post or its topic in this 
 thread*! Use the post in the Announce forum for that:

 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org


 I believe Mike meant that this thread should be used for 
 discussion of
 the DIP, while the other thread should be used for discussion 
 of the
 blog post/OB/ live.


Exactly.

Sasha.
Jul 15 2019
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/15/2019 1:31 PM, Max Haughton wrote:

 I think this DIP should at least consider expanding its scope to use the 
 reaching definition analysis.
 
 It seems slightly pointless if, even if it's already opt in, that we can
bypass 
 it so easily e.g. by assigning a temporary.
 
 Or is it really *that* complicated to allow the compiler to analyse this?


This is what will happen in a later proposal which will do comprehensive data 
flow analysis per function.
Aug 02 2019
parent reply Olivier FAURE <couteaubleu gmail.com> writes:
On Friday, 2 August 2019 at 08:09:22 UTC, Walter Bright wrote:

 On 7/15/2019 1:31 PM, Max Haughton wrote:

 I think this DIP should at least consider expanding its scope 
 to use the reaching definition analysis.
 
 It seems slightly pointless if, even if it's already opt in, 
 that we can bypass it so easily e.g. by assigning a temporary.
 
 Or is it really *that* complicated to allow the compiler to 
 analyse this?


 This is what will happen in a later proposal which will do 
 comprehensive data flow analysis per function.


So... still not addressing the safety holes pointed out in above 
comments?

Or is it coming in the next DIP iteration?
Aug 03 2019
parent Exil <Exil gmall.com> writes:
On Saturday, 3 August 2019 at 09:52:24 UTC, Olivier FAURE wrote:

 On Friday, 2 August 2019 at 08:09:22 UTC, Walter Bright wrote:

 On 7/15/2019 1:31 PM, Max Haughton wrote:

 I think this DIP should at least consider expanding its scope 
 to use the reaching definition analysis.
 
 It seems slightly pointless if, even if it's already opt in, 
 that we can bypass it so easily e.g. by assigning a temporary.
 
 Or is it really *that* complicated to allow the compiler to 
 analyse this?


 This is what will happen in a later proposal which will do 
 comprehensive data flow analysis per function.


 So... still not addressing the safety holes pointed out in 
 above comments?

 Or is it coming in the next DIP iteration?


It's obvious where this is going, it's going to get added without 
a thought out complete DIP like DIP1000. This review process is 
just a charade.
Aug 03 2019
prev sibling next sibling parent SashaGreat <sasha gmail.com> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, Mike Parker wrote:

 ...
 But *please do not discuss the blog post or its topic in this 
 thread*! Use my post in the Announce forum for that:
 .
 .
 .
 I will be moving (copying, deleting, and pasting in a new 
 thread) posts that do not adhere to the reviewer guidelines. So 
 please keep all comments in this thread focused on DIP 1021.


I don't get it.
Jul 15 2019
prev sibling next sibling parent rikki cattermole <rikki cattermole.co.nz> writes:
I had written up a comment all about the DIP.
And then I reread it.

This line seems to be very important and I missed it the first time: 
"The checks would only be enforced for  safe code.".

If the semantics are indeed only valid in  safe code, the single example 
in the DIP does not show this to be the case.

It would be nice to have some pseudo-code based rules like the Rust one.
Just to make it clear in addition to some worked examples as to what is 
going on valid/invalid wise and how it ties into DIP25 and DIP1000.
Jul 15 2019
prev sibling next sibling parent Dukc <ajieskola gmail.com> writes:
You might want to add to the rationale section that this DIP 
should help the compiler to produce better optimizations, as it 
does not need to check for pointer aliasing.
Jul 16 2019
prev sibling next sibling parent reply Elronnd <elronnd elronnd.net> writes:
Will this be able to detect delegates which close over variables? 
  Consider this snippet (which, incidentally, also serves as a POC 
for  safe since it compiles successfully under  safe):

byte bb; //must be global so its address can be taken
void foo(ref byte *b, void delegate()  safe fun) {
         fun();
         *b = 4;
}
void test() {
         byte *s = &bb;

         foo(s, {s = null;});
}


It seems like a bit of an intractable problem to detect.
Jul 16 2019
parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/16/2019 2:25 PM, Elronnd wrote:

 It seems like a bit of an intractable problem to detect.


Not really. It's doable.
Jul 16 2019
prev sibling next sibling parent Dominikus Dittes Scherkl <dominikus.scherkl continental-corporation.com> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, 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


+1

I don't mind the braking change. Functions taking multiple 
mutable references are always suspect, so marking them  trusted 
or  system is a good idea anyway.
Jul 17 2019
prev sibling next sibling parent reply Exil <Exil gmall.com> writes:
What is even being reviewed here? The DIP isn't ready for review 
obviously. It doesn't specify or define how anything is going to 
work, there is nothing to review!

It prompts replies like this:

On Wednesday, 17 July 2019 at 01:49:49 UTC, Walter Bright wrote:

 On 7/16/2019 2:25 PM, Elronnd wrote:

 It seems like a bit of an intractable problem to detect.


 Not really. It's doable.


Okay, how is it doable, explain how it will be doable. I feel 
like any explanation that is given by the author won't be 
thoroughly thought out and will contain obvious holes in the 
explanation. The blog post mentions  live, there's nothing about 
that in the DIP. Not sure if you can even call this a "review". I 
feel this is just another DIP that will be rushed through to the 
finish line where it will be accepted irregardless of the DIP.



Anyways. There's no examples. No possible workarounds. Missing 
details for basically everything.

Per the description of the DIP:


 The checks would only be enforced for  safe code.


Yet the rationale only gives an example using C's free(). Which 
can't be used in  safe.
Jul 17 2019
parent Olivier FAURE <couteaubleu gmail.com> writes:
On Wednesday, 17 July 2019 at 12:30:34 UTC, Exil wrote:

 What is even being reviewed here? The DIP isn't ready for 
 review obviously. It doesn't specify or define how anything is 
 going to work, there is nothing to review!


I agree with your points, but I think you could have worded them 
better. As it is, you come across as a little rude and abrasive.

Reviews should be ruthless, but diplomatic.
Jul 17 2019
prev sibling next sibling parent reply Olivier FAURE <couteaubleu gmail.com> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, 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

 All review-related feedback on and discussion of the DIP should 
 occur in this thread. The review period will end at 11:59 PM ET 
 on July 30, or when I make a post declaring it complete.


I want to strongly state that this DIP is too vague and 
unpolished to justify its integration to the language standard, 
provided it's being judged by the same standards as previous DIPs.

In fact, this DIP reminds me a lot of DIP 1016 (the "rvalue ref" 
one), in that it suffers from similar problems.

To quote my post-rejection analysis[1] of DIP 1016:


 Now, you might say (and have said in the past) "Well that's 
 obvious, you just need to do X, unless Y, in which case Z", but 
 the point is that all these considerations *need to be in the 
 proposal*.

 As it is, the proposal doesn't have any negative space. That 
 is, it explains what the feature should look like, but it 
 doesn't define boundaries, where the feature is or isn't 
 allowed to be used.

 Defining negative space is important because it's how you find 
 corner cases, cases where the correct behavior seems obvious, 
 and yet when you spell it out it turns out everyone disagrees 
 on how it should be handled.

 Right now, the DIP doesn't address potential corner cases [at 
 all].


In particular, one area in which this proposal is lacking is 
examples.

Now, Walter has expressed that a spec document isn't made any 
clearer by a large number of examples, or by having more complex 
examples, and I mostly agree.

But the problem with the current examples is that they don't 
cover any changes in behavior introduced by the proposal, let 
alone corner cases. There should be at least one example showing 
 safe code that compiles before DIP 1021, and gives an error 
message after the proposal. The example would show a simplistic 
RAII-style container, with  safe or  trusted getters and setters; 
something like:

     struct S1 {
         byte* ptr;

      safe:
         ~this() { reset(); }
         ref byte get() { return *ptr; }

      trusted:
         void init() { ptr = cast(byte*)malloc(1); *ptr = 0; }
         void reset() { free(ptr); ptr = null; }
     }

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), and it doesn't even show what a syntax error would 
look like.

Ideally, there should also be additional examples illustrating 
nuances of the proposal; eg, since the proposal is meant to apply 
to ref and not pointers, there should be examples showing code 
that compiles with pointers and doesn't compile with refs.

Examples aside, this proposal is also way too vague and informal. 
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? Are you allowed to do this:

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

To quote Andrei's post-rejection analysis[2] of DIP 1016:


 It has multiple problems of informality and vague language that 
 have worked together to cause said misunderstanding. [...]

 The DIP must convince the reader, and in a way the reader does 
 not "owe" the DIP. [...] Of course we don't want to be as harsh 
 as academics could get, but we don't want to transform DIP 
 acceptance into a farmers market bargaining process.

 [...] In a thorough submission, however, there would have been 
 many places that clear that up:

 * Use of a distinct notation (non-code non-text font for 
 metalanguage, i.e. general expressions);

 * Description of the typechecking process, with examples of 
 code that passes and code that fails;

 * Several places in text in which it is explained that rvalues 
 resulted from implicit conversions are not eligible;


I think a lot of advice in Andrei's review is good advice for 
writing any DIP, and I think it absolutely applies to this one.

[1]: 
https://forum.dlang.org/post/hxqjetbtzlzzaxnpwgyk forum.dlang.org
[2]: https://forum.dlang.org/post/q2u429$1cmg$1 digitalmars.com
Jul 17 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
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.
Jul 17 2019
next sibling parent reply Olivier FAURE <couteaubleu gmail.com> writes:
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.
Jul 18 2019
parent reply Nick Treleaven <nick geany.org> writes:
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.
Jul 18 2019
parent reply Olivier FAURE <couteaubleu gmail.com> writes:
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.
Jul 18 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/18/2019 4:31 AM, Olivier FAURE wrote:

 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,


The only one mutable pointer XOR one or more const pointers is not a difficult 
concept. A sufficient description of it is included in the DIP, and a link 
provided for those who want to learn more.

Also, the DIP does not say "we're going to use a memory model like the one Rust 
has."



 and doesn't explain how various corner cases will 
 be handled.


Please enumerate them.

Keep in mind that the argument tracking code is already in the compiler, it was 
needed for DIP25 and DIP1000. This is pointed out in the DIP.



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


Right, it's restricted to function calls.
Jul 18 2019
parent Olivier FAURE <couteaubleu gmail.com> writes:
On Friday, 19 July 2019 at 01:24:24 UTC, Walter Bright wrote:

 On 7/18/2019 4:31 AM, Olivier FAURE wrote:

 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,


 The only one mutable pointer XOR one or more const pointers is 
 not a difficult concept. A sufficient description of it is 
 included in the DIP, and a link provided for those who want to 
 learn more.


Also, please don't be condescending to me.

Yes, "one mutable pointer XOR one or more const pointers" is 
simple. It's not what this DIP is proposing.
Jul 19 2019
prev sibling parent reply Exil <Exil gmall.com> writes:
On Wednesday, 17 July 2019 at 23:26:20 UTC, Walter Bright wrote:

 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.


You are not limited to one example. Showing an implementation 
using a common RAII pattern to illustrate why it isn't sufficient 
and why it requires a new feature to replace/complement it. RAII 
is part of the reason why you should never be calling free 
directly to begin with. And the borrowing feature from rust very 
closely resembles std::unique_ptr from C++.


 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.


Showing an example that doesn't use malloc/free or something 
similar to it, would be more beneficial then. As malloc/free 
generally is replaced by RAII and isn't/shouldn't be used 
directly anyways.

How does the feature differentiate between GC and non-gc 
allocated pointers? If I label one function  live won't this 
limit all pointer operations, even though it is only really 
relevant to non-gc pointers? This seems like two different 
features clashing against one another, both trying to do the same 
thing but interfering with one another.
Jul 18 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/18/2019 4:54 PM, Exil wrote:

 You are not limited to one example. Showing an implementation using a common 
 RAII pattern to illustrate why it isn't sufficient and why it requires a new 
 feature to replace/complement it.


One example is all that's necessary. Adding a bunch of irrelevant code to it 
adds nothing.

What part of the proposal are you not understanding?

(If one doesn't understand that simple example, a far more complex one won't
help.)



 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.


 
 Showing an example that doesn't use malloc/free or something similar to it, 
 would be more beneficial then.


Any container that does a realloc() when something is added to it will trigger 
the problem.



 As malloc/free generally is replaced by RAII and 
 isn't/shouldn't be used directly anyways.


It's irrelevant if it is used directly or indirectly, as the result is the
same. 
Since RAII is irrelevant to the issue, adding an RAII object would only serve
to 
confuse the reader into thinking it is about RAII.

In fact, the DIP is not necessary if people strictly adhere to RAII.



 How does the feature differentiate between GC and non-gc allocated pointers?


It doesn't.



 If I label one function  live won't this limit all pointer operations, even
though 
 it is only really relevant to non-gc pointers?


 live is a separate proposal.
Jul 18 2019
next sibling parent reply David Bennett <davidbennett bravevision.com> writes:
On Friday, 19 July 2019 at 01:32:34 UTC, Walter Bright wrote:

 One example is all that's necessary. Adding a bunch of 
 irrelevant code to it adds nothing.

 What part of the proposal are you not understanding?


For me it's unclear if the examples as given would become compile 
time errors because it states "The checks would only be enforced 
for  safe code." and the examples are not  safe.

Would the examples give errors in  system code or are they just 
there to illustrate buggy code and the idea being these could be 
made  safe with future work? (or both?)

Also, does it also check simple non-heap references, for example 
would this become compile time error?

---
 safe void f(scope ref int a, scope ref int b)
{
      a=1;
      assert(b==1);
}
 safe void main()
{
     int i = 0;
     f(i, i);
     assert(i==1);
}
---

Horrible code, but it compiles and runs with -dip1000 -dip25 
currently. https://run.dlang.io/is/XdQBzJ

If it's going to be an error, a simple example like this in the 
Breaking Changes and Deprecations section could help clear up 
whats changing from a practical sense.
Jul 18 2019
next sibling parent reply David Bennett <davidbennett bravevision.com> writes:
On Friday, 19 July 2019 at 02:59:59 UTC, David Bennett wrote:

 Also, does it also check simple non-heap references, for 
 example would this become compile time error?

 ---
  safe void f(scope ref int a, scope ref int b)
 {
      a=1;
      assert(b==1);
 }
  safe void main()
 {
     int i = 0;
     f(i, i);
     assert(i==1);
 }
 ---


Actually re-reading the DIP again it's says "multiple mutable 
references to the same object" not "multiple references to the 
same mutable object" so I'm guessing my example above would still 
compile with -dip1021 ?

If that's so I'm guessing this would be the simplest thing that 
would stop compiling?

---
 safe void f(scope ref int* a, scope ref int* b)
{
     *a=1;
     a = null;
     assert(*b==1);

}
 safe void main()
{
     int i = 0;
     scope int* a = &i;
     scope int* b = &i;
     f(a, b);
     assert(i==1);
     assert(a is null);
     assert(*b==1);
}
---
Jul 18 2019
parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/18/2019 8:31 PM, David Bennett wrote:

 If that's so I'm guessing this would be the simplest thing that would stop 
 compiling?
 
 ---
  safe void f(scope ref int* a, scope ref int* b)
 {
      *a=1;
      a = null;
      assert(*b==1);
 
 }
  safe void main()
 {
      int i = 0;
      scope int* a = &i;
      scope int* b = &i;
      f(a, b);
      assert(i==1);
      assert(a is null);
      assert(*b==1);
 }
 ---


That will still compile, because as the DIP says, it only looks at the function 
call, it is not doing data flow analysis on preceding or succeeding statements.
Jul 19 2019
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/18/2019 7:59 PM, David Bennett wrote:

 For me it's unclear if the examples as given would become compile time errors 
 because it states "The checks would only be enforced for  safe code." and the 
 examples are not  safe.


Any container is going to wrap a call to free() in  trusted.



 Also, does it also check simple non-heap references, for example would this 
 become compile time error?
 
 ---
  safe void f(scope ref int a, scope ref int b)
 {
       a=1;
       assert(b==1);
 }
  safe void main()
 {
      int i = 0;
      f(i, i);
      assert(i==1);
 }
 ---


Yes, because you are passing two mutable references to the same memory.



 If it's going to be an error, a simple example like this in the Breaking
Changes 
 and Deprecations section could help clear up whats changing from a practical
sense.


Since the DIP disallows passing two mutable pointers to the same memory object, 
any code that does will break.
Jul 19 2019
parent David Bennett <davidbennett bravevision.com> writes:
On Friday, 19 July 2019 at 08:01:50 UTC, Walter Bright wrote:

 On 7/18/2019 7:59 PM, David Bennett wrote:


 Also, does it also check simple non-heap references, for 
 example would this become compile time error?
 
 ---
  safe void f(scope ref int a, scope ref int b)
 {
       a=1;
       assert(b==1);
 }
  safe void main()
 {
      int i = 0;
      f(i, i);
      assert(i==1);
 }
 ---


 Yes, because you are passing two mutable references to the same 
 memory.

 ...

 Since the DIP disallows passing two mutable pointers to the 
 same memory object, any code that does will break.


Thanks for clearing that up, it was not obvious what a "mutable 
pointer" was in that context as in this example only the data 
appeared to be mutable and not the pointer.

With that in mind would passing 2 slices to overlaping data cause 
a compile time error?

---
 safe void f(scope int[] a, scope int[] b)
{
     a[1] = 1;
     assert(b[0]==1);

}
 safe void main()
{
     int[] i = new int[32];
     f(i[0..$], i[1..$]);
     assert(i[1]==1);
}
---

Sorry for all the questions, I believe this DIP or something link 
it is a good step in the right direction. I'm just trying to 
update my mental model of D so I can write code with this in mind 
today and have a better understanding.
Jul 21 2019
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/18/2019 6:32 PM, Walter Bright wrote:

 On 7/18/2019 4:54 PM, Exil wrote:

 You are not limited to one example. Showing an implementation using a common 
 RAII pattern to illustrate why it isn't sufficient and why it requires a new 
 feature to replace/complement it.


 
 One example is all that's necessary. Adding a bunch of irrelevant code to it 
 adds nothing.


But what the heck. Open the file:

https://github.com/dlang/dmd/blob/master/src/dmd/root/outbuffer.d

Look at line 408, peekSlice(). It returns a slice of data[].

Now look at line 132, writeByte(). It calls reserve(1). Line 50 reallocates 
data[], invalidating the pointer returned by peekSlice().

Write a function that passes both the OutBuffer reference, and the result of 
peekSlice(). In that function, call writeByte(). Boom.

The example in the DIP is the same thing, minus the 470 lines of other stuff.
Jul 19 2019
next sibling parent reply Olivier FAURE <couteaubleu gmail.com> writes:
On Friday, 19 July 2019 at 07:59:14 UTC, Walter Bright wrote:

 https://github.com/dlang/dmd/blob/master/src/dmd/root/outbuffer.d

 Look at line 408, peekSlice(). It returns a slice of data[].

 Now look at line 132, writeByte(). It calls reserve(1). Line 50 
 reallocates data[], invalidating the pointer returned by 
 peekSlice().

 Write a function that passes both the OutBuffer reference, and 
 the result of peekSlice(). In that function, call writeByte(). 
 Boom.


Alright, I'm going to cut to what I think is the heart of the 
problem.

     void someFunction()
     {
         auto buffer = OutBuffer(someData);
         auto slice = buffer.peekSlice();

         buffer.writeByte();
         slice[0] = 42;          // Boom
     }

As you're aware, nothing about your DIP stops the above case from 
happening. So I don't understand what the point of this DIP even 
is, given that for every memory leak that you your DIP prevents, 
I could give you another way to express it so that your DIP lets 
it pass.

It doesn't close a loophole in  safe, because the core problem is 
that writeByte() isn't actually  safe; presumably, your function 
that takes the OutBuffer reference and the result of peekSlice() 
wouldn't be  safe either, so it wouldn't even be stopped by your 
DIP.


 The example in the DIP is the same thing, minus the 470 lines 
 of other stuff.


Nobody is asking you to write a 470-lines example. People are 
asking you to write at least one example that would behave 
differently with your DIP than it behaves now.

Like, you insist on that, like the fact that we're asking for 
examples means we don't understand your DIP and thus we're not 
fit to judge it, or like we're being unreasonable and we're only 
asking because we're being capricious or something.

Speaking personally, I'm not asking for examples because I don't 
understand your proposal. The reason I'm asking for examples is 
that examples are a good medium to support communication, and to 
suss out the deeper reasons people might disagree about an idea.

I'm pretty sure I do understand your proposal, and I think it's 
fundamentally flawed. I think you're trying to gradually 
implement the Rust memory model, and you think that non-aliased 
function arguments are a good first step, that will help 
determine how a full implementation should be rolled out. I think 
that approach is unworkable, because the Rust memory model is 
kind of an all-or-nothing feature, and gradually implementing it 
like you intend isn't really feasible; I think DIP 1021, if it 
passes, will barely be noticed by anyone, except as an annoyance, 
and won't bring any useful feedback for designing a more coherent 
memory model. Because, again, DIP 1021 doesn't actually bring any 
additional memory safety, because "use after free" in real code 
almost never occurs in a way that could be stopped by DIP 1021.
Jul 19 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/19/2019 1:49 AM, Olivier FAURE wrote:

 So I don't understand what the point of this DIP even is, given that for every 
 memory leak that you your DIP prevents, I could give you another way to
express 
 it so that your DIP lets it pass.
 It doesn't close a loophole in  safe, because the core problem is that 
 writeByte() isn't actually  safe;


Any container that does its own memory allocation relies on  trusted code.

However, if that container exposes its internals by ref, then it is safe with 
this DIP, because ref does not span statements.



 Speaking personally, I'm not asking for examples because I don't understand
your 
 proposal. The reason I'm asking for examples is that examples are a good
medium 
 to support communication, and to suss out the deeper reasons people might 
 disagree about an idea.


It would have facilitated communication if you'd acknowledged you understood
the 
example, but had a question about another specific case (and presented that 
example).



 I'm pretty sure I do understand your proposal, and I think it's fundamentally 
 flawed. I think you're trying to gradually implement the Rust memory model,
and 
 you think that non-aliased function arguments are a good first step, that will 
 help determine how a full implementation should be rolled out. I think that 
 approach is unworkable, because the Rust memory model is kind of an 
 all-or-nothing feature, and gradually implementing it like you intend isn't 
 really feasible; I think DIP 1021, if it passes, will barely be noticed by 
 anyone, except as an annoyance, and won't bring any useful feedback for 
 designing a more coherent memory model.


D has been gradually implementing that memory model for some time now - DIP25 
and DIP1000 are examples. This is the next logical step.



 Because, again, DIP 1021 doesn't 
 actually bring any additional memory safety, because "use after free" in real 
 code almost never occurs in a way that could be stopped by DIP 1021.


It's a workable solution for containers that expose pointers to their internals 
via "ref", because "ref" is a restricted pointer. I've advocated for years that 
people use "ref" when exposing internals.
Jul 19 2019
next sibling parent reply Olivier FAURE <couteaubleu gmail.com> writes:
On Friday, 19 July 2019 at 20:03:22 UTC, Walter Bright wrote:

 On 7/19/2019 1:49 AM, Olivier FAURE wrote:

 So I don't understand what the point of this DIP even is, 
 given that for every memory leak that you your DIP prevents, I 
 could give you another way to express it so that your DIP lets 
 it pass.
 It doesn't close a loophole in  safe, because the core problem 
 is that writeByte() isn't actually  safe;


 Any container that does its own memory allocation relies on 
  trusted code.

 However, if that container exposes its internals by ref, then 
 it is safe with this DIP, because ref does not span statements.


Counter-example:

     struct S1 {
         byte* ptr;

      safe:
         ~this() { reset(); }
         ref byte get() return { return *ptr; }

      trusted:
         void init() { ptr = cast(byte*)malloc(1); *ptr = 0; }
         void reset() { free(ptr); ptr = null; }
     }

     byte* identity(ref return byte b)  safe {
         return &b;
     }

     void useS()  safe {
         S1 s = S1();
         s.init();

         byte* ptr = identity(s.get());
         s.reset();
         *ptr = 1; // BOOM
     }


 Speaking personally, I'm not asking for examples because I 
 don't understand your proposal. The reason I'm asking for 
 examples is that examples are a good medium to support 
 communication, and to suss out the deeper reasons people might 
 disagree about an idea.


 It would have facilitated communication if you'd acknowledged 
 you understood the example, but had a question about another 
 specific case (and presented that example).


I presented that specific problem 22 days ago: 
https://github.com/dlang/DIPs/pull/158#issuecomment-506633378

You said it didn't apply to your case, which is why I asked you 
to demonstrate your case. You said your example already did.

I (and several other people) pointed out a non-subjective problem 
with your example: it doesn't compile any differently before and 
after the DIP as presented. Which I have repeated several times, 
and you still haven't acknowledged.

I'm going to repeat that one more time: your example does not 
compile *any differently* before or after the proposal as you 
describe it, ***and I'd appreciate if could acknowledge it in any 
way at all***.

So its power to facilitate communication is kind of null.

Yes, I can make guesses at what you intend. This was, you'll 
recall, the same argument Manu made for DIP 1016, that the 
underlying concepts were clear enough, and that there was no need 
for giving specific semantics because anyone should understand 
the general concept (you can make references to rvalues).

That's not how a review process works. A reviewer shouldn't have 
to prove their worth before the submitter includes a single valid 
example in their proposal (which, I'll note, you still haven't 
done). Reviewers shouldn't have to write up the examples 
themselves like we did and conjecture about whether they match 
the author's intent.

Like, I'm not sure that you realize, but the process of 
communicating with you is kind of painful and makes me want not 
to bother at all. You seem to take anybody asking you for 
additional details as something between a personal insult and an 
annoying waste of time. In fact, you've spent more time arguing 
about not writing examples than it took me to write a single 
example.

In fact, in the three weeks you've been arguing that writing 
examples was a waste of your time, I've written four different 
examples trying to understand your proposal.

I'm honestly kind of done here. I'm getting sick of putting 
effort into understanding your proposal and trying to communicate 
with you that you're neither matching nor even acknowledging. 
I'll just work on my own draft and leave this DIP alone.
Jul 19 2019
parent reply Nick Treleaven <nick geany.org> writes:
On Friday, 19 July 2019 at 22:36:40 UTC, Olivier FAURE wrote:

     byte* identity(ref return byte b)  safe {
         return &b;
     }


When I saw this (last week), I was surprised it compiled, but it 
does (with -dip1000). The returned pointer is scope, but that has 
a different lifetime from ref returns, which only last for the 
line that called the function.

I don't think `return &b` should compile (but even with that 
change, this DIP doesn't make exposing heap memory as ref work 
with  safe).
Jul 25 2019
parent Nick Treleaven <nick geany.org> writes:
On Thursday, 25 July 2019 at 12:03:19 UTC, Nick Treleaven wrote:

 On Friday, 19 July 2019 at 22:36:40 UTC, Olivier FAURE wrote:

     byte* identity(ref return byte b)  safe {
         return &b;
     }


 When I saw this (last week), I was surprised it compiled, but 
 it does (with -dip1000). The returned pointer is scope, but 
 that has a different lifetime from ref returns, which only last 
 for the line that called the function.

 I don't think `return &b` should compile


https://issues.dlang.org/show_bug.cgi?id=20183
Aug 30 2019
prev sibling parent Nick Treleaven <nick geany.org> writes:
On Friday, 19 July 2019 at 20:03:22 UTC, Walter Bright wrote:

 However, if that container exposes its internals by ref, then 
 it is safe with this DIP, because ref does not span statements.


Timon has disproved this:
https://forum.dlang.org/post/qgsroo$1l8n$1 digitalmars.com

With that struct container as a member of a class, you can have 
multiple class references to the same class instance and pass one 
of those to a function with a ref argument of the struct-owned 
heap memory.
Jul 20 2019
prev sibling parent aliak <something something.com> writes:
On Friday, 19 July 2019 at 07:59:14 UTC, Walter Bright wrote:

 Write a function that passes both the OutBuffer reference, and 
 the result of peekSlice(). In that function, call writeByte(). 
 Boom.

 The example in the DIP is the same thing, minus the 470 lines 
 of other stuff.


writeByte and peekSlice are both  system and can't be called from 
 safe code though. That won't compile today. What did I miss?
Jul 19 2019
prev sibling parent Exil <Exil gmall.com> writes:
On Friday, 19 July 2019 at 01:32:34 UTC, Walter Bright wrote:

 In fact, the DIP is not necessary if people strictly adhere to 
 RAII.


Bingo. This is why RAII needs to be mentioned, otherwise this DIP 
is not necessary.
Jul 19 2019
prev sibling next sibling parent reply ag0aep6g <anonymous example.com> writes:
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/793f83911fdc8c88c6ef34e6a36b5e11e3e5
4e5/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.
Jul 18 2019
next sibling parent reply Nick Treleaven <nick geany.org> writes:
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).
Jul 18 2019
next sibling parent Nick Treleaven <nick geany.org> writes:
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);
Jul 18 2019
prev sibling parent reply ag0aep6g <anonymous example.com> writes:
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.
Jul 18 2019
parent Nick Treleaven <nick geany.org> writes:
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.
Jul 18 2019
prev sibling parent aliak <something something.com> writes:
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
Jul 18 2019
prev sibling next sibling parent reply Mike Franklin <slavo5150 yahoo.com> writes:
Consider this contrived example:

someLibrary.d
-------------
module someLibrary;

import core.stdc.stdlib;

int a;
int b;

ref int foo()  safe
{
     if ((rand() % 2) == 0)
     	return a;
     else
     	return b;
}

ref int bar()  safe
{
     if ((rand() % 4) == 0)
     	return a;
     else
     	return b;
}

main.d
------
import someLibrary;

void doSomething(scope ref int a, scope ref int b)  safe
{
     // whatever...
}

void main()  safe
{
     doSomething(foo(), bar());
}

How can the compiler statically determine whether `foo` and `bar` 
will return a reference to the same data?

It seems the language or type system must provide this guarantee, 
which would require a full-fledged ownership/borrowing system to 
accurately enforce what this DIP is proposing.
Jul 19 2019
next sibling parent aliak <something something.com> writes:
On Friday, 19 July 2019 at 08:32:13 UTC, Mike Franklin wrote:

 void main()  safe
 {
     doSomething(foo(), bar());
 }

 How can the compiler statically determine whether `foo` and 
 `bar` will return a reference to the same data?

 It seems the language or type system must provide this 
 guarantee, which would require a full-fledged 
 ownership/borrowing system to accurately enforce what this DIP 
 is proposing.


 From what I understand it won't with this dip. foo and bar do not 
refer to the same "memory object"
Jul 19 2019
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/19/2019 1:32 AM, Mike Franklin wrote:

 How can the compiler statically determine whether `foo` and `bar` will return
a 
 reference to the same data?


It can't.



 It seems the language or type system must provide this guarantee, which would 
 require a full-fledged ownership/borrowing system to accurately enforce what 
 this DIP is proposing.


A full fledged one disallows accessing mutable global state, and now you know 
why :-)

(Because the function signature does not tell the complete story when the 
function accesses mutable global state.)
Jul 21 2019
parent reply Mike Franklin <slavo5150 yahoo.com> writes:
On Sunday, 21 July 2019 at 09:34:42 UTC, Walter Bright wrote:

 On 7/19/2019 1:32 AM, Mike Franklin wrote:

 How can the compiler statically determine whether `foo` and 
 `bar` will return a reference to the same data?


 It can't.



 It seems the language or type system must provide this 
 guarantee, which would require a full-fledged 
 ownership/borrowing system to accurately enforce what this DIP 
 is proposing.


 A full fledged one disallows accessing mutable global state, 
 and now you know why :-)


But it's the same issue even without the global state:

--- someLibrary.d
module someLibrary;

import core.stdc.stdlib;

struct S
{
     int a = 1;
     int b = 2;

     ref int foo()  safe
     {
         if ((rand() % 2) == 0)
             return a;
         else
             return b;
     }

     ref int bar()  safe
     {
         if ((rand() % 4) == 0)
             return a;
         else
             return b;
     }
}

--- main.d
import someLibrary;

void doSomething(scope ref int a, scope ref int b)  safe
{
     // whatever...
}

void main()  safe
{
     S s;
     for(int i = 0; i < 100; i++)
     	doSomething(s.foo(), s.bar());
}

I still don't see how the compiler can know statically whether or 
not `foo()` and `bar()` will return a reference to the same data.


 (Because the function signature does not tell the complete 
 story when the function accesses mutable global state.)


That's why a full ownership/borrowing system seems necessary 
because with such a system (either through the type system or the 
language's semantics or both), there is only ever one mutable 
reference to any data, and therefore, attribution of functions 
and/or function parameters is not required.

It appears a full ownership/borrowing system would make what this 
DIP is proposing obsolete, and is the only way to properly do 
what this DIP is proposing.

Mike
Jul 21 2019
next sibling parent reply Olivier FAURE <couteaubleu gmail.com> writes:
On Monday, 22 July 2019 at 00:05:12 UTC, Mike Franklin wrote:

 import core.stdc.stdlib;

 struct S
 {
     int a = 1;
     int b = 2;

     ref int foo()  safe
     {
         if ((rand() % 2) == 0)
             return a;
         else
             return b;
     }

     ref int bar()  safe
     {
         if ((rand() % 4) == 0)
             return a;
         else
             return b;
     }
 }

 ...

 I still don't see how the compiler can know statically whether 
 or not `foo()` and `bar()` will return a reference to the same 
 data.


Assuming you compile with dip1000 and dip1021, foo and bar in the 
above code would have to be marked `return` (that is, `this` is a 
`return ref`) or trigger a compile error.

The compiler would presumably be conservative and assumes that 
they always return references to overlapping data, which is 
usually what you want (it's what the Rust compiler does, at 
least).
Jul 22 2019
parent Mike Franklin <slavo5150 yahoo.com> writes:
On Monday, 22 July 2019 at 08:45:11 UTC, Olivier FAURE wrote:


 Assuming you compile with dip1000 and dip1021, foo and bar in 
 the above code would have to be marked `return` (that is, 
 `this` is a `return ref`) or trigger a compile error.


Indeed. That seems to work.


 The compiler would presumably be conservative and assumes that 
 they always return references to overlapping data, which is 
 usually what you want (it's what the Rust compiler does, at 
 least).


I'd rather not make any assumptions.  Walter, what say you?  
Given the code below (with added `return` lifetime annotations), 
how would DIP1021 behave?

--- someLibrary.d
module someLibrary;

import core.stdc.stdlib;

struct S
{
     int a = 1;
     int b = 2;

     ref int foo()  safe return
     {
         if ((rand() % 2) == 0)
             return a;
         else
             return b;
     }

     ref int bar()  safe return
     {
         if ((rand() % 4) == 0)
             return a;
         else
             return b;
     }
}

--- main.d
import someLibrary;

void doSomething(scope ref int a, scope ref int b)  safe
{
     // whatever...
}

void main()  safe
{
     S s;
     for(int i = 0; i < 100; i++)
     	doSomething(s.foo(), s.bar());
}

Thanks,
Mike
Jul 22 2019
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/21/2019 5:05 PM, Mike Franklin wrote:

 void main()  safe
 {
      S s;
      for(int i = 0; i < 100; i++)
          doSomething(s.foo(), s.bar());
 }
 
 I still don't see how the compiler can know statically whether or not `foo()` 
 and `bar()` will return a reference to the same data.


Because they're both returning a reference to `s`. The compiler already checks 
this with dip25 and dip1000. This DIP just extends this to more checking.


 It appears a full ownership/borrowing system would make what this DIP is 
 proposing obsolete, and is the only way to properly do what this DIP is
proposing.


A full OB requires the  live sections. This DIP makes it partially available in 
 safe sections, enough so containers can control access to their components.
Jul 31 2019
prev sibling next sibling parent reply Nick Treleaven <nick geany.org> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, Mike Parker wrote:

 https://github.com/dlang/DIPs/blob/793f83911fdc8c88c6ef34e6a36b5e11e3e574e5/DIPs/DIP1021.md


OK, I think I have a practical example that this DIP enables. 
Here is a RAII  nogc dynamic array that can be appended to, and 
exposes individual elements by ref. Before the destructor and 
`append` could not be marked  trusted because of `maybe_bad` - 
its ref parameter `i` may have outlived a.data.ptr. Now all 
methods of Array can be  trusted AFAIK.

struct Array
{
     private int[] data;
     import core.stdc.stdlib;
      nogc:

     void append(int e)  trusted
     {   // inefficient but it works
         const len = data.length + 1;
         auto ptr = cast(int*)realloc(data.ptr, len * int.sizeof);
         data = ptr[0..len];
     }

     ref int get(size_t i)  trusted return
     {
         return data[i];
     }

     ~this()  trusted
     {
         if (data.ptr) free(data.ptr);
     }
}

 safe:

void maybe_bad(ref Array a, ref int i)
{
     a = Array(); // frees a.data.ptr
     i++;
}

void main()
{
     Array a;
     a.append(5);
     // compiler knows result of `get` is owned by `a`
     // error: Can't pass multiple mutable references owned by `a`
     maybe_bad(a, a.get(0));
}

Walter - is this correct?
Jul 19 2019
next sibling parent Nick Treleaven <nick geany.org> writes:
On Friday, 19 July 2019 at 13:09:29 UTC, Nick Treleaven wrote:

     void append(int e)  trusted
     {   // inefficient but it works
         const len = data.length + 1;
         auto ptr = cast(int*)realloc(data.ptr, len * 
 int.sizeof);
         data = ptr[0..len];
     }


Oops, forgot last line:

         data[$-1] = e;
Jul 19 2019
prev sibling next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 19.07.19 15:09, Nick Treleaven wrote:

 On Monday, 15 July 2019 at 15:23:32 UTC, Mike Parker wrote:

 https://github.com/dlang/DIPs/blob/793f83911fdc8c88c6ef34e6a36b5e11e3e5
4e5/DIPs/DIP1021.md 


 
 OK, I think I have a practical example that this DIP enables. Here is a 
 RAII  nogc dynamic array that can be appended to, and exposes individual 
 elements by ref. Before the destructor and `append` could not be marked 
  trusted because of `maybe_bad` - its ref parameter `i` may have 
 outlived a.data.ptr. Now all methods of Array can be  trusted AFAIK.
 
 struct Array
 {
      private int[] data;
      import core.stdc.stdlib;
       nogc:
 
      void append(int e)  trusted
      {   // inefficient but it works
          const len = data.length + 1;
          auto ptr = cast(int*)realloc(data.ptr, len * int.sizeof);
          data = ptr[0..len];
      }
 
      ref int get(size_t i)  trusted return
      {
          return data[i];
      }
 
      ~this()  trusted
      {
          if (data.ptr) free(data.ptr);
      }
 }
 
  safe:
 
 void maybe_bad(ref Array a, ref int i)
 {
      a = Array(); // frees a.data.ptr
      i++;
 }
 
 void main()
 {
      Array a;
      a.append(5);
      // compiler knows result of `get` is owned by `a`
      // error: Can't pass multiple mutable references owned by `a`
      maybe_bad(a, a.get(0));
 }
 
 Walter - is this correct?


Some problems with this code that don't relate to the DIP:
- It still has a default constructor, so you can call `Array(new 
int[](10))` and cause a free on a GC pointer.
- The copy constructor is neither implemented nor disabled, so you can 
easily cause a double free.


I don't think the DIP enables any more code to be  trusted (as it points 
out in the limitations section).

Consider:

 safe:
class C{
     Array a;
     this(Array a){ this.a=move(a); }
}

void maybe_bad(C c, ref int i){
     c.a = Array();
     i++;
}

void main(){
     auto c=new C(Array());
     c.a.append(5);
     auto d=c;
     maybe_bad(c,d.a.get());
}
Jul 19 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/19/2019 9:39 AM, Timon Gehr wrote:

  safe:
 class C{
      Array a;
      this(Array a){ this.a=move(a); }
 }
 
 void maybe_bad(C c, ref int i){
      c.a = Array();
      i++;
 }
 
 void main(){
      auto c=new C(Array());
      c.a.append(5);
      auto d=c;
      maybe_bad(c,d.a.get());
 }


If Array was ref-counted, then it will be safe. The problem with ref-counted 
objects, as you pointed out years ago, is this:

     maybe_bad(c,c.a.get());

Copying c to d will increase the ref count and so it will be safe.

Without this DIP, ref-counted objects can't be safe. With it, they can.
Jul 31 2019
next sibling parent Olivier FAURE <couteaubleu gmail.com> writes:
On Wednesday, 31 July 2019 at 09:25:26 UTC, Walter Bright wrote:

 On 7/19/2019 9:39 AM, Timon Gehr wrote:

  safe:
 class C{
      Array a;
      this(Array a){ this.a=move(a); }
 }
 
 void maybe_bad(C c, ref int i){
      c.a = Array();
      i++;
 }
 
 void main(){
      auto c=new C(Array());
      c.a.append(5);
      auto d=c;
      maybe_bad(c,d.a.get());
 }


 Copying c to d will increase the ref count and so it will be 
 safe.


You misread the above example.

c is an instance of a class holding an Array, not an instance of 
an Array. So copying c to d doesn't involve calling Array's copy 
constructor.
Jul 31 2019
prev sibling parent Nick Treleaven <nick geany.org> writes:
On Wednesday, 31 July 2019 at 09:25:26 UTC, Walter Bright wrote:

 Copying c to d will increase the ref count and so it will be 
 safe.


As Olivier said, c and d are just class references to the same 
class instance, so there is only one instance of Array, a ref 
count would still be 1.

This problem can also happen with slices, here's a simplified 
struct that shows the problem and disables the struct postblit:

struct S
{
     private byte* data;
     import core.stdc.stdlib;
      nogc:

     this(byte b)  trusted
     {
         data = cast(byte*)malloc(1);
         *data = b;
     }
      disable this(this);

     ref byte get()  trusted return
     {
         return *data;
     }

     ~this()  trusted
     {
         if (data) free(data);
     }
}

 safe:

void maybe_bad(ref S s, ref byte i)
{
     s = S(); // frees s.data
     i++;
}

void main()
{
     auto s = S(5);
     // DIP error: Can't pass multiple mutable references owned by 
`s`
     maybe_bad(s, s.get());
     //auto t = s; // S is not copyable

     auto c = new C;
     c.s = S(4);
     auto d = c;
     // not detected by DIP
     maybe_bad(c.s, d.s.get());

     auto a = [S(3)];
     auto b = a;
     // not detected by DIP
     maybe_bad(a[0], b[0].get());
}

class C
{
     S s;
}
Jul 31 2019
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/19/2019 6:09 AM, Nick Treleaven wrote:

 Walter - is this correct?


Yup.
Jul 31 2019
parent Olivier FAURE <couteaubleu gmail.com> writes:
On Wednesday, 31 July 2019 at 07:09:49 UTC, Walter Bright wrote:

 On 7/19/2019 6:09 AM, Nick Treleaven wrote:

 Walter - is this correct?


 Yup.


After Nick posted this example, Timon posted this refutal:
https://forum.dlang.org/post/qgsroo$1l8n$1 digitalmars.com

The general argument being that Nick's code isn't any more  safe 
with DIP-1021 than it is now, since even with DIP-1021 you can 
still use Array to corrupt memory in  safe code.

What is your take on it?
Jul 31 2019
prev sibling next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
Implementation:

https://github.com/dlang/dmd/pull/10249
Jul 31 2019
prev sibling next sibling parent nkm1 <t4nk074 openmailbox.org> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, 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 problem with this DIP is that it's just a small part of some 
bigger thing, and it looks like no one understands this bigger 
thing except maybe Walter. It also seems that some people don't 
believe that Walter actually knows what the end result will be. 
Maybe he's just making it up as he goes along?
Anyway, all of this confirms my opinion - D is a language 
supposed to work with GC, and all these contortions are just 
trying to fit a square peg into a round hole.
Just use Rust instead.
Aug 31 2019
prev sibling next sibling parent reply Mike Franklin <slavo5150 yahoo.com> writes:
On Monday, 15 July 2019 at 15:23:32 UTC, 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

 All review-related feedback on and discussion of the DIP should 
 occur in this thread. The review period will end at 11:59 PM ET 
 on July 30, or when I make a post declaring it complete.

 At the end of Round 2, if further review is deemed necessary, 
 the DIP will be scheduled for another round of Community 
 Review. Otherwise, it will be queued for the Final Review and 
 Formal Assessment.

 For some additional context on this DIP, please see Walter's 
 latest post on the D Blog titled "Ownership and Borrowing in D":

 https://dlang.org/blog/2019/07/15/ownership-and-borrowing-in-d/

 But *please do not discuss the blog post or its topic in this 
 thread*! Use my post in the Announce forum for that:

 https://forum.dlang.org/post/hetwxvibgqcmoltvnoda forum.dlang.org

 Anyone intending to post feedback in this thread is expected to 
 be familiar with the reviewer guidelines:

 https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

 I will be moving (copying, deleting, and pasting in a new 
 thread) posts that do not adhere to the reviewer guidelines. So 
 please keep all comments in this thread focused on DIP 1021.

 Thanks in advance to all who participate.


I recently discovered a programming language called "Pony" 
(https://www.ponylang.io/) and I believe it has an interesting 
take on the pointer aliasing problem that is quite different than 
Rust and is more akin to how D addresses such problems (e.g. 
through attribution).

Pony uses what they call "Reference Capabilities" to tell the 
compiler how references can or cannot be aliased, and I think it 
is relevant to the same problem that this DIP and other efforts 
within D are trying to solve.

You can find more information here:
https://tutorial.ponylang.io/reference-capabilities
https://zartstrom.github.io/pony/2016/08/28/reference-capabilities-in-pony.html
https://www.youtube.com/watch?v=HGDSnOZaU7Y&t=1869s

I hope you find it interesting if not influential.

Mike
Sep 11 2019
parent jmh530 <john.michael.hall gmail.com> writes:
On Wednesday, 11 September 2019 at 09:37:38 UTC, Mike Franklin 
wrote:

 [snip]

 I recently discovered a programming language called "Pony" 
 (https://www.ponylang.io/) and I believe it has an interesting 
 take on the pointer aliasing problem that is quite different 
 than Rust and is more akin to how D addresses such problems 
 (e.g. through attribution).

 Pony uses what they call "Reference Capabilities" to tell the 
 compiler how references can or cannot be aliased, and I think 
 it is relevant to the same problem that this DIP and other 
 efforts within D are trying to solve.

 You can find more information here:
 https://tutorial.ponylang.io/reference-capabilities
 https://zartstrom.github.io/pony/2016/08/28/reference-capabilities-in-pony.html
 https://www.youtube.com/watch?v=HGDSnOZaU7Y&t=1869s

 I hope you find it interesting if not influential.

 Mike


I was under the impression that some early Rust work was similar 
to Pony before they switched things up.

I think the reference capability that is most relevant to D is 
the iso one. That lines up with Oliver Faure's scheme to add a 
unique qualifier [1] I think.

https://forum.dlang.org/post/fgrxchwtltrvgpnfmagt forum.dlang.org
Sep 11 2019
prev sibling parent Yuxuan Shui <yshuiv7 gmail.com> writes:
Ownership/lifetime is definitely a very useful feature for D. 
However, what is purposed by this DIP seems to be very limited. I 
wonder how useful this will actually be in practice. (That been 
said, since we could easily iteratively improve on top of this 
DIP, this is not really a problem).

It would be nice if this DIP could include some real world bugs 
that would have been caught by this form of ownership/lifetime 
analysis.
Sep 15 2019