Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Extend cases where expressions are passed by move [skip ci] #11686

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nordlow
Copy link
Contributor

@nordlow nordlow commented Sep 4, 2020

This enables more situations where variables and parameters are automatically passed by move. It works by detecting when all the references to a VarDeclaration can be passed by move. For those cases move is triggered by setting STC.rvalue of the VarDeclaration.

The first goal here is to enable a lazy range to be constructed with a non-copyable container type as its source ranges, typically, as

this(R)(R source)
{
    this.source = source; // `source` can be passed by move here
}

. This case can be activated by a simple special-casing in inferStatementRvalues without the current added book-keepings

  • VarExp.vrefOrder
  • VarDeclaration.vrefCount
  • VarDeclaraiton.wasMovedBy

which are currently there for experimenting purposes.

There book-keepings are here because they enable more pass-by-move cases. The question is if we should try enabling these as well.

There's now also a possibility to enable better error messages by giving the location of the VarExp that already moved a VarDeclaration if more than one move occurs for the same VarDeclaration.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @nordlow! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11686"

@nordlow nordlow force-pushed the more-moves branch 3 times, most recently from 6cd8f44 to b2a0a61 Compare September 4, 2020 22:36
@nordlow nordlow changed the title WIP: Extend cases where expressions are passed by move [avoid ci] WIP: Extend cases where expressions are passed by move [skip ci] Sep 4, 2020
@Geod24
Copy link
Member

Geod24 commented Sep 5, 2020

Taking a quick look, I'm not 100% certain I understand exactly how you plan to achieve this.

Bear in mind that anything that requires caller-callee coordination cannot be done "magically", because of separate compilation. Even for templates, the compiler will not emit templates that it knows are already instantiated in the root module that defines the template.

The first goal here is to enable a lazy range to be constructed with a non-copyable container type as its source ranges, typically, as [...]

Why not use ref here ? Or a pointer.

@nordlow nordlow force-pushed the more-moves branch 2 times, most recently from 2e77530 to c4393ea Compare September 7, 2020 08:39
@nordlow
Copy link
Contributor Author

nordlow commented Sep 7, 2020

Why not use ref here ? Or a pointer.

Pointer is not @safe. ref cannot be assignment to range source member. For reference, in Rust this feature is into_iter.

@andralex has approved this move-enhancement here dlang/phobos#4971.

@RazvanN7
Copy link
Contributor

RazvanN7 commented Jan 8, 2021

Currently, there is a move DIP in the DIP queue that is authored by @WalterBright . I suspect that part of this implementation is going to be part the implementation of the DFA that identifies where to insert calls to the move constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants