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