Skip to content

Copying, Moving and Forwarding#182

Merged
mdparker merged 18 commits intodlang:masterfrom
WalterBright:13NNN-WGB.md
Mar 3, 2021
Merged

Copying, Moving and Forwarding#182
mdparker merged 18 commits intodlang:masterfrom
WalterBright:13NNN-WGB.md

Conversation

@WalterBright
Copy link
Member

No description provided.

@anass-O
Copy link

anass-O commented Feb 22, 2020

last use is moved

Please no. This should not be implicit and just makes it harder to reason about code and what is happening. This is an invisible difference, and unexpected. If the user already isn't taking care and moving the object or using references then odds are this isnt going to provide any significant performance benefit but its going to be much more difficult to understand what is happening cognitively. An objects destruct is no longer at the end of scope but at the last place it was passed to a function, where you then have to check, was it used after that function or not. Then let's not forget about scope(exit) that uses the object, if that's placed at the start then you shouldn't move the object in the last function call. Figuring out whether a move is or isnt going to happen is too complicated and not worth the benefit at all.

@radcapricorn
Copy link

radcapricorn commented Feb 29, 2020

@anass-O

An objects destruct is no longer at the end of scope but at the last place it was passed to a function

It is at end of scope. Of the last user of the object. The whole point is exactly to elide unnecessary calls, not enforce a C++ dogma of when destruction occurs. If one needs specific code to execute at end of scope, one should use a scope(exit), not a destructor.

where you then have to check, was it used after that function or not.

You don't have to. Compiler would have to, and it should have to, and this is something that's been missing from D for absolute ages for no good reason (other than figuring out the implementation, of course).

int dtor_calls;

struct T {
    ~this() { dtor_calls += 1; }
}

struct S {
    T payload;
    this(T data) {
        // data is passed by value, should be destructed at end of scope...
        payload = data; 
        // ...except why would you insert a dtor call here if all you needed was to move
        // the guts of `data` into `payload`? Moved-from objects should be in a state that
        // does not necessitate any destruction!
    }
}

void main() {
    {
        S s = T.init;
    }
    assert(dtor_calls == 1);
}

The above assertion should pass. In current D it doesn't and it can't, unless this proposal is implemented.

In current D it is simply impossible to express an efficient move past the topmost call, and one has to forcibly copy bits via the std.algorithm.mutation.move function (or its facade the std.functional.forward). This is wasteful, especially given a compiler that (should be) capable of eliding unnecessary copies.
Library implementation of move function(s) is an atrocity that in an ideal world should be abolished, or at the very least replaced with a compiler intrinsic (which is what C++ does as there it's a mere cast).

Then let's not forget about scope(exit) that uses the object, if that's placed at the start then you shouldn't move the object in the last function call.

And you won't. Nor would the compiler, exactly because the object is still in use at the closing brace. The "Multiple last accesses" example from the proposal illustrates the same.

Figuring out whether a move is or isnt going to happen is too complicated and not worth the benefit at all.

It isn't complicated in the slightest. Last use by value - move; pretty straightforward. Moreover, it isn't something a programmer would normally be even worrying about, except in one specific case: asserting ownership. It's impossible to do that in current D. With this proposal, it will be - owned objects would have move ctors implemented and copy ctors disabled, which should suffice for static enforcement of ownership.

The swap example from the proposal is a wonderful illustration of its practicality. No, necessity. Look at the current implementation of std.algorithm.mutation.swap. All that gunk just to avoid unnecessary copy ctor and dtor calls.

@WalterBright
An issue I see with the proposal (unless I'm missing something) is the apparent inability to explicitly express a move while still retaining use of the object (for example, to surrender ownership but emplace new contents). The "Assignment after move" section comes close but I don't like the discretionary power of the implementation there. I'd say that in case of assignment following pass by value that pass must be a move followed by a blit of a .init, destruction pre-assignment elided. Or heck, a move followed by a blit of a .init and the appropriate ctor (not opAssign)? :) But that's probably best left for review.


### Class Objects

Class objects cannot have move constructors or move assignment operators.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please expand upon this, with reasoning here. If it has something to do with being reference type, then mention said reason.

Copy link

@anass-O anass-O left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radcapricorn

@anass-O

An objects destruct is no longer at the end of scope but at the last place it was passed to a function

It is at end of scope. Of the last user of the object. The whole point is exactly to elide unnecessary calls, not enforce a C++ dogma of when destruction occurs. If one needs specific code to execute at end of scope, one should use a scope(exit), not a destructor.

It destructs an empty object at end of scope. The actual objects contents is moved at its last use. So the actual contents of the object, that you actually care about, is now destructed within that called function. This is most definitely changing when an object gets destructed. When an empty object that's never used gets destructed is irrelevant.

where you then have to check, was it used after that function or not.

You don't have to. Compiler would have to, and it should have to, and this is something that's been missing from D for absolute ages for no good reason (other than figuring out the implementation, of course).

If you need to understand what the code is actually doing yes you do. Compilers don't write code, programmers do and they need to be able to understand what that code is doing. Like I said, the complexity added to understand what the code is doing far outweighs the insignificant performance gain the change would get.

int dtor_calls;

struct T {
    ~this() { dtor_calls += 1; }
}

struct S {
    T payload;
    this(T data) {
        // data is passed by value, should be destructed at end of scope...
        payload = data; 
        // ...except why would you insert a dtor call here if all you needed was to move
        // the guts of `data` into `payload`? Moved-from objects should be in a state that
        // does not necessitate any destruction!
    }
}

void main() {
    {
        S s = T.init;
    }
    assert(dtor_calls == 1);
}

The above assertion should pass. In current D it doesn't and it can't, unless this proposal is implemented.

It wouldn't pass even with this DIP. There's 2 destructors being called. One for the S.payload member variable. The second for the parameter data. That would still be the case with this DIP. Even if you use "ref" to bind the rvalue to a reference. The second object that would need to be destroyed would just be in a different scope.

So much for it not being "complicated" 🤪.

In current D it is simply impossible to express an efficient move past the topmost call, and one has to forcibly copy bits via the std.algorithm.mutation.move function (or its facade the std.functional.forward). This is wasteful, especially given a compiler that (should be) capable of eliding unnecessary copies.
Library implementation of move function(s) is an atrocity that in an ideal world should be abolished, or at the very least replaced with a compiler intrinsic (which is what C++ does as there it's a mere cast).

I'm fully aware how awful D is for moving. I never said moving shouldn't be added. This DIP, this is not the way.

Then let's not forget about scope(exit) that uses the object, if that's placed at the start then you shouldn't move the object in the last function call.

And you won't. Nor would the compiler, exactly because the object is still in use at the closing brace. The "Multiple last accesses" example from the proposal illustrates the same.

Again if you want to read and understand the code, yea you do. Compilers don't write code, programmers do. They need to understand what is happening otherwise programmers wouldn't be needed.

Figuring out whether a move is or isnt going to happen is too complicated and not worth the benefit at all.

It isn't complicated in the slightest. Last use by value - move; pretty straightforward. Moreover, it isn't something a programmer would normally be even worrying about, except in one specific case: asserting ownership. It's impossible to do that in current D. With this proposal, it will be - owned objects would have move ctors implemented and copy ctors disabled, which should suffice for static enforcement of ownership.

When you have thousands of lines of code, with dozens of objects, it isnt going to be as simple as a 4 line example is making it out to be. If you want to understand what the code is doing then yes you do. Write code for programmers, not compilers. This will undoubtedly lead to those kinds of bugs you spend hours trying to figure out. Just to realize the compiler was doing unnecessary magic on an object that would provide almost no performance gain.

The swap example from the proposal is a wonderful illustration of its practicality. No, necessity. Look at the current implementation of std.algorithm.mutation.swap. All that gunk just to avoid unnecessary copy ctor and dtor calls.

You'd just be moving that into every object to manage it on their own. Your not removing the gunk, your just moving and spreading it elsewhere.

@WalterBright
An issue I see with the proposal (unless I'm missing something) is the apparent inability to explicitly express a move while still retaining use of the object (for example, to surrender ownership but emplace new contents). The "Assignment after move" section comes close but I don't like the discretionary power of the implementation there. I'd say that in case of assignment following pass by value that pass must be a move followed by a blit of a .init, destruction pre-assignment elided. Or heck, a move followed by a blit of a .init and the appropriate ctor (not opAssign)? :) But that's probably best left for revie

```

The call to `fun` is the last access of `s` if and only if all the statements following it,
up to `<statement_n>` do not access `s` at all.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording needs to be fixed. Otherwise as it is currently worded this will be a bug, unless this is the intended behavior:

void f()
{
    S s;
    scope(exit) {
        fun2(s);
    }

    fun1(s); // last use?
    // no stmt_n here, fun1(s) would be considered last use (but it's not, scope(exit) is last use)
}

I don't agree with this feature. But if your going to put a turd on the table, it might as well be a shiny turd 💩.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed literally at the start of the section.

or write is done to the memory region referred to directly by the lvalue. The last use
of variable declarations is identified by doing dataflow analysis on function local
variables and function parameters that are passed by value. Global variables and `ref`
parameters are not checked for last use. For example, for a given function `f`:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How accurate is this going to be?

struct S {
   // emo (nice name btw)
}

struct T {
S* s;
extern(C++) S* getProp() { return s; } // what if this isn't visible to do detection
}

void f() {
    S s;
    S* sp;
    {
        auto t = T(&s);
        sp = t.getProp();
    }

    scope(exit) {
         fun(*sp);
    }

    fun(s); // is this considered last use by the compiler (even though it actually isn't)?
}

This would need a lot of code analysis to determine when last use actually is.

Disclaimer: I don't agree with this feature. But if you are going to put a turd on the table, it might as well be a shiny turd 💩.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This situation is explicitly addressed in the DIP. You're taking the address of s.

Copy link

@anass-O anass-O Mar 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not addressed in the DIP. Your taking an address, within a nested scope. That scope ends, the address it took is escaped. There's no way for data flow analysis to be able to catch unless it is able to see the getProp() code.

Go ahead and tell me how it should be implemented then, the actual implementation of the compiler. You probably won't be able to as it seems you don't understand what the DIP is actually suggestioning to do. You just have that some abstract idea of it, which is why you think this situation is covered simply be cause there's a section about pointers.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Whenever the address of a variable x is taken, x will lose the possibility of last access optimization." Unless dictionaries were rewritten recently, "whenever" means what it says.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are ways around that as well. The amount of rules this is introducing is just crazy. And Walter was trying to use simplifying the language to remove some features. I understand why now, cause he plans to introduce all of this rats nest of complexity. What C++ has implemented is far better than this, it is much simpler and the user can actually reason about the code, and move the object when they want to. Not under some very limited and narrow circumstances, and only the last use at that.

@radcapricorn
Copy link

@anass-O

@radcapricorn

@anass-O

An objects destruct is no longer at the end of scope but at the last place it was passed to a function

It is at end of scope. Of the last user of the object. The whole point is exactly to elide unnecessary calls, not enforce a C++ dogma of when destruction occurs. If one needs specific code to execute at end of scope, one should use a scope(exit), not a destructor.

It destructs an empty object at end of scope.

It does not. Read the DIP.

int dtor_calls;

struct T {
    ~this() { dtor_calls += 1; }
}

struct S {
    T payload;
    this(T data) {
        // data is passed by value, should be destructed at end of scope...
        payload = data; 
        // ...except why would you insert a dtor call here if all you needed was to move
        // the guts of `data` into `payload`? Moved-from objects should be in a state that
        // does not necessitate any destruction!
    }
}

void main() {
    {
        S s = T.init;
    }
    assert(dtor_calls == 1);
}

The above assertion should pass. In current D it doesn't and it can't, unless this proposal is implemented.

It wouldn't pass even with this DIP. There's 2 destructors being called. One for the S.payload member variable. The second for the parameter data. That would still be the case with this DIP. Even if you use "ref" to bind the rvalue to a reference. The second object that would need to be destroyed would just be in a different scope.

With this DIP the assert must pass. data won't be destructed, because payload is move-constructed from it (which it can be since data is not used after that). The only time a T dtor would be called in the above code is when s goes out of scope.

So much for it not being "complicated" 🤪.

Not complicated at all.

I'm fully aware how awful D is for moving. I never said moving shouldn't be added. This DIP, this is not the way.

So far you haven't presented any valid arguments as to why "this is not the way", and are actually actively demonstrating that you haven't even fully read the proposal. All of your comments so far are addressed by the DIP already.

@anass-O
Copy link

anass-O commented Mar 4, 2020

@radcapricorn

@anass-O

@radcapricorn

@anass-O

An objects destruct is no longer at the end of scope but at the last place it was passed to a function

It is at end of scope. Of the last user of the object. The whole point is exactly to elide unnecessary calls, not enforce a C++ dogma of when destruction occurs. If one needs specific code to execute at end of scope, one should use a scope(exit), not a destructor.

It destructs an empty object at end of scope.

It does not. Read the DIP.

Perhaps you should?

https://github.com/dlang/DIPs/pull/182/files#diff-6dee50bbd8ade48660f68be4943b957bR408

If passed by Move ref, then the caller is responsible for destructing the object. If the intention is to treat Move ref differently, then that's even worse and it adding even more complexity about what the code is actually doing. Right now when see a function call and you see the signature, you know exactly what it is doing. That isn't going to be the case anymore, this adds a bunch of confusing rules that the compiler won't be able to uphold through data flow analysis. Rust is able to do it because it was designed from the start to be able to. This will only really be possible if D effectively implements a borrow checker to the degree that Rust does. This feature would then thus be limited to the scope of @safe and @live (tbd), but this is not mentioned anywhere in the DIP. I don't see how this is feasible at all in @system code. If you don't have source to do data flow analysis, for any reason, such as calling C++ code. Or using a library that just uses .di header files. Then it won't be able to accurately calculate last use. What does it do then?

I'm fully aware how awful D is for moving. I never said moving shouldn't be added. This DIP, this is not the way.

So far you haven't presented any valid arguments as to why "this is not the way", and are actually actively demonstrating that you haven't even fully read the proposal. All of your comments so far are addressed by the DIP already.

The added complexity is the reason why. The other reason why, is that data flow analysis actually requires the code to be able to do data flow analysis. Just cause you point to something in the DIP, doesn't mean that an actual feasible implementation of what you are suggesting exists in the real world. Rust is able to implement this because of the borrow checker, you have to follow rules of the borrow checker, which are really strict. This DIP makes no such mention.

I don't know how Walter can be so against introducing some sort of simple type for string interpolation, all the while having this mess of a DIP that introduces something as convoluted as EMO objects. And rely on data flow analysis that the compiler simply won't be able to do. This is where concrete implementations are better than DIPs that promise things it simply won't be able to deliver in the real world.

@radcapricorn
Copy link

@radcapricorn

@anass-O

@radcapricorn

@anass-O

An objects destruct is no longer at the end of scope but at the last place it was passed to a function
It is at end of scope. Of the last user of the object. The whole point is exactly to elide unnecessary calls, not enforce a C++ dogma of when destruction occurs. If one needs specific code to execute at end of scope, one should use a scope(exit), not a destructor.

It destructs an empty object at end of scope.
It does not. Read the DIP.

Perhaps you should?

https://github.com/dlang/DIPs/pull/182/files#diff-6dee50bbd8ade48660f68be4943b957bR408

And? What does

void func(ref S);

which is what that section you're quote-mining is referring to, have to do with

void func(S);

which is what this DIP is all about???
Of course in the former case the caller is responsible for the destruction. When calling a void func(ref S), no move is performed.

If passed by Move ref, then the caller is responsible for destructing the object.

That's not what that section talks about. At all.

If the intention is to treat Move ref differently, then that's even worse...

Move ref has nothing to do with your quote mine. You can't bind an rvalue (or a hypothetical Move ref) to a ref. Ergo, attempting to call a void func(ref S) as func(S()) will not compile.

I was going to respond to the rest, but after the above I really see no point. I'd rather spend my time more productively while waiting for actual discussion/feedback, if/when it comes to that.

@anass-O
Copy link

anass-O commented Mar 5, 2020

@radcapricorn
That's a convient way to dismiss the most relevant part of the comment :). I understand why Walter only responds to one sentence now.

@radcapricorn
Copy link

@anass-O
The most relevant part of your comment(s) is that you're arguing against this DIP without understanding it, as demonstrated. Thus, you're making up irrelevant excuses.

No complex flow analysis is required. You're passing a local by value. Are you accessing it after that? If concrete "no", you may move it and elide destruction. If the following access is unconditional assignment to that local, you may still move it but can't elide destruction just yet. In all other cases you don't move it. That's it.
"All other cases" include taking the address of that local.
You don't need access to source of the functions you're calling. All you need are signatures.

Rust's borrow checker has nothing to do with this. It tracks references and prevents overlapping lifetimes of "mutable references" (which should be called references to mutable, but whatever). This is not what this DIP is about, at all. It's about values and, specifically, rvalues.

1. A: `s` is copied, B: `s` is assigned
2. A: `s` is moved, B: `s` gets constructed, not assigned

at the implementation's discretion. Case 1 can be done for a quick build,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this should compile:

struct S {
    // non-copyable
    @disable this(ref S);
    // EMO
    this(S other) { /* ... */ }
    void opAssign(S other) { /* ... */ }
}

S createNewS();
void consume(S);

void usage() {
    S s = createNewS();
    consume(s); // cannot be copied
    s = createNewS();
    consume(s); // cannot be copied
    // s is not destructed here
}

This precludes such discretionary power of the implementation. It would have to treat pass before a = as a move-then-construct. But, if so defined, this may make = a bit confusing:

void usage2() {
    auto s = createNewS();
    s = createNewS(); // move-assign
    consume(s); // move
    s = createNewS(); // move-construct
    // s is destructed here
}

@jmh530
Copy link

jmh530 commented Jun 5, 2020

You say

Ironically, because move constructors execute arbitrary code, this likely would interfere with the collector keeping track of the state of memory during a collection cycle, meaning that objects with move constructors would likely be pinned (treated as immoveable).

Should this be true for all objects with move constructors, or just those that do not use the default move constructor? You wouldn't want all struct objects to be immoveable, no?

@12345swordy
Copy link

12345swordy commented Jan 17, 2021

Who going to be this dip champion, as Walter is no longer allowed to write and evaluate his own dip?
@WalterBright do you have anyone in mind to be your dip champion?

@mdparker
Copy link
Member

@12345swordy Still looking for one.

@12345swordy
Copy link

@mdparker what are the requirements for the DIP champion anyways?

@Bolpat
Copy link
Contributor

Bolpat commented Jan 20, 2021

@andralex would be an obvious candidate.

@mdparker
Copy link
Member

@12345swordy The most important is a solid understanding of the proposal and its ramifications. The sponsor will completely take over the DIP and decide if and how to revise it through the review process. So a thorough understanding is important. That and a willingness to see the whole thing through.

However, this DIP (and the other Walter has in the Draft queue) was submitted before the decision was made about DIPs from the maintainers. That decision was intended for future DIPs that hadn't yet been written. Given that these were (and are) in Draft status, we could have gone either way with them: let them continue on under Walter or try to find someone to take them over. We decided to give some time for the latter. But if we can't find anyone, then I'm going to move them through under Walter's authorship.

But we aren't ready for that yet, and I would prefer not to go that route. So if anyone is interested in taking over either of them, please do let me know.

@mdparker mdparker merged commit 39fa253 into dlang:master Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants