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

librustc: Implement the unboxed closure trait hierarchy. #17543

Closed

Conversation

pcwalton
Copy link
Contributor

This makes FnMut require FnOnce and Fn require FnMut. Therefore,
this change breaks code that implements the FnMut and/or Fn traits
directly, without also implementing their dependencies. A simple
forwarding implementation that defines FnOnce in terms of FnMut
and/or Fn in terms of FnMut will suffice.

This does not affect code that simply uses the |&:|/|&mut:|/|:|
unboxed closure construction notation.

Part of RFC #44; needed to implement RFC #63.

Part of issue #12831.

[breaking-change]

r? @nikomatsakis

This makes `FnMut` require `FnOnce` and `Fn` require `FnMut`. Therefore,
this change breaks code that implements the `FnMut` and/or `Fn` traits
directly, without also implementing their dependencies. A simple
forwarding implementation that defines `FnOnce` in terms of `FnMut`
and/or `Fn` in terms of `FnMut` will suffice.

This does not affect code that simply uses the `|&:|`/`|&mut:|`/`|:|`
unboxed closure construction notation.

Part of RFC rust-lang#44; needed to implement RFC rust-lang#63.

Part of issue rust-lang#12831.

[breaking-change]
@nikomatsakis
Copy link
Contributor

OK, so, I read through this patch and it all makes sense. However, I also remembered that what I originally had in mind was that we could (I think) alternatively create an impl for all FnMut in terms of Fn and so on (impl<A,R,T:Fn<A,R>> FnMut<A,R> for T) and hence have no actual extra work to do in the compiler. Of course this requires PR #17669 to work, since it is an example of conditional dispatch. Unless there is some reason this won't work that is escaping me at the moment.

@nikomatsakis
Copy link
Contributor

@pcwalton the aliasing restrictions don't matter to trans...but by-value self is potentially different LLVM bitcode, yes.

@nikomatsakis
Copy link
Contributor

@pcwalton I just drew up a quick sketch showing what I mean -- it seems to work, at least when all the impls are user-defined: 82d37da

Hat tip: @glaebhoerl suggested this approach first.

@alexcrichton
Copy link
Member

ping @pcwalton, any progres here?

@nikomatsakis
Copy link
Contributor

This should be fairly trivial to do now. I'm going to check it out.

@nikomatsakis
Copy link
Contributor

Closing in favor of #18388

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.

3 participants