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

RFC: Add DerefMove #178

Closed
wants to merge 6 commits into from
Closed

RFC: Add DerefMove #178

wants to merge 6 commits into from

Conversation

ftxqxd
Copy link
Contributor

@ftxqxd ftxqxd commented Jul 23, 2014

@lilyball
Copy link
Contributor

👍

* Something is being dereferenced, and the only `Deref*` trait it implements is
`DerefMove`;
* Something is being dereferenced, and while it *does* implement `Deref` and/or
`DerefMut`, it also implements `DerefMove` and a value is being moved out of
Copy link
Member

Choose a reason for hiding this comment

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

I feel like I am missing something in this second bullet: Whether we choose to move or copy a value is based on the static type of that value's expression. If I implement Deref<C> on Foo (for some C:Copy), then *foo will copy rather than move, right? Therefore, this specification seems circular, e.g. if I also implement DerefMove<Box<()>> on Foo as well, then I need to know the type of an expression like *foo to know whether its return value is being copied or being moved, but in order to know what the type of *foo is, I need to know whether I am supposed to be invoking deref or deref_move.

But perhaps I have misunderstood your intention, and there is some other property that you are trying to describe here that is unrelated to the type-based move/copy distinction. And if that is the case, perhaps there is another phrasing that would be clearer here. (I'm just guessing now, but would "if the dereference is in an rvalue context" work?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I mention a ‘move’ in the RFC, I generally mean a ‘move or copy’. So deref_move is called (if is implemented) whenever a value is moved or copied out of a dereference. This means that if I had a struct, Foo, that implemented Deref<int>, *foo (where foo: Foo) would call deref (because Foo doesn’t implement DerefMove). However, if I implemented DerefMove<uint> on Foo as well, even a copy like *foo would call deref_move, not deref any more.

In other words, deref_move is called if it looks like a move or copy. On types that also implement DerefMove, deref and deref_mut are only called when an expression along the lines of &*p or &mut *p is encountered.

Copy link
Contributor

Choose a reason for hiding this comment

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

That won't work. uint is Copy but Foo probably isn't. It would need to call Deref<int> on Foo and copy out of the resulting &int instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s a good point. Maybe the rules could be revised to this:

  1. If the type implements DerefMove but not Deref or DerefMut, all dereferences are done using deref_move.
  2. If a reference is being taken to a dereference (e.g. &*ptr), call deref or deref_mut, depending on the mutability of the reference.
  3. If a value is being dereferenced and implements DerefMove and Deref and/or DerefMut:
    1. If the type is Copy, call deref_move.
    2. Otherwise:
      1. If the type implements Deref<T> where T: Copy, call *deref or *deref_mut.
      2. Otherwise, call deref_move.
  4. If a value is being dereferenced and does not implement DerefMove but does implement Deref and/or DerefMut, use deref or deref_mut.

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems reasonable to me. Presumably if the type implements both Deref and DerefMut, it will choose to use deref instead of deref_mut in 3.ii.a and 4.

@nikomatsakis
Copy link
Contributor

Thanks for the RFC. I believe the plan is to implement DerefMove eventually. However, the design I had in mind is somewhat different from the one you sketched out. In particular, I planned to require that

trait DerefMove<T> : Deref<T> { ... }

This has the downside that Cell<T> cannot implement DerefMove<T>, but I think it is important for type checking. This is because we must be able to determine the type of an expression like *x before we know whether what precise kind of deref it is. Therefore, we need to be able to always use Deref<T> to start and then go fix it up later. I'm also quite disturbed by the idea of a type implementing both Deref<T> and DerefMove<U>, where T != U (which would otherwise be permitted).

3. If a value is being dereferenced and implements `DerefMove` and `Deref`:
1. If the type is `Copy`, call `deref_move`.
2. Otherwise:
1. If the type implements `Deref<T>` where `T: Copy`, call `*deref`.
Copy link

Choose a reason for hiding this comment

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

It's not clear to me what is actually mean here - is this supposed to be *x.deref()?

@jdm
Copy link

jdm commented Nov 17, 2014

I'm concerned about making DerefMove inherit from Deref. Servo needs DerefMove to allow us to create JSRef values on demand that borrow the Root that is being dereferenced. We're hacking around the currently lack of DerefMove by returning a JSRef value that isn't actually valid in all cases; there's no value we could return for Deref that would make sense in a world with DerefMove.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Nov 26, 2014

I guess the best way (from a user perspective) of resolving the issue with DerefMove inheriting from Deref would be to greatly extend where clauses and trait bounds to be much more powerful, giving the ability to say that a DerefMove<T> implementor must not implement Deref<U> where T is not U:

// A hypothetical syntax for the above
pub trait DerefMove<T>: for<U> !Deref<U> where T != U {
    // ...
}

(Pretend the type parameters are associated types, like they should be.) I’m not sure if this would have any other repercussions relating to coherence.

@tomjakubowski
Copy link
Contributor

(meta-comment: could you update the Rendered View link in the top comment?)

@nikomatsakis
Copy link
Contributor

This is an important topic, but not one we are prepared to address now in the runup to 1.0. I hope we can come back to it shortly thereafter. Postponing for now under issue #997.

@nikomatsakis
Copy link
Contributor

Oh, and in case it wasn't clear, thanks very much for the RFC!

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
Annotate `CpuFuture` with `#[must_use]`.
@anchpop
Copy link

anchpop commented May 12, 2022

Should this issue be reopened now that the original rationale for closing it seems to no longer apply? I think there is quite a bit of interest.

@clarfonthey
Copy link
Contributor

If you look slightly above your comment, that is tracked in #997.

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

Successfully merging this pull request may close these issues.

9 participants