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

Blanket impls for &F/&mut F where F: Fn/F:FnMut respectively #23895

Merged

Conversation

nikomatsakis
Copy link
Contributor

The primary purpose of this PR is to add blanket impls for the Fn traits of the following (simplified) form:

impl<F:Fn> Fn for &F
impl<F:FnMut> FnMut for &mut F

However, this wound up requiring two changes:

  1. A slight hack so that x() where x: &mut F is translated to FnMut::call_mut(&mut *x, ()) vs FnMut::call_mut(&mut x, ()). This is achieved by just autoderef'ing one time when calling something whose type is &F or &mut F.
  2. Making the infinite recursion test in trait matching a bit more tailored. This involves adding a notion of "matching" types that looks to see if types are potentially unifiable (it's an approximation).

The PR also includes various small refactorings to the inference code that are aimed at moving the unification and other code into a library (I've got that particular change in a branch, these changes just lead the way there by removing unnecessary dependencies between the compiler and the more general unification code).

Note that per rust-lang/rfcs#1023, adding impls like these would be a breaking change in the future.

cc @japaric
cc @alexcrichton
cc @aturon

Fixes #23015.

… and onto the

`UnificationTable`, and renaming/collapsing some methods.
trait matching more tailored. We now detect recursion where the
obligations "match" -- meaning basically that they are the same for some
substitution of any unbound type variables.
F`, so that if we have `x: &mut FnMut()`, then `x()` is translated to
`FnMut::call_mut(&mut *x, ())` rather than `&mut x`. The latter would
require `mut x: &mut FnMut()`, which is really a lot of mut. (Actually,
the `mut` is normally required except for the special case of a `&mut F`
reference, because that's the one case where we distinguish a unique
path like `x` from a mutable path.)
@rust-highfive
Copy link
Collaborator

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Nice!

@pnkfelix
Copy link
Member

pnkfelix commented Apr 1, 2015

@bors r+ 11111bb

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 1, 2015
…dd-impls, r=pnkfelix

The primary purpose of this PR is to add blanket impls for the `Fn` traits of the following (simplified) form:

    impl<F:Fn> Fn for &F
    impl<F:FnMut> FnMut for &mut F

However, this wound up requiring two changes:

1. A slight hack so that `x()` where `x: &mut F` is translated to `FnMut::call_mut(&mut *x, ())` vs `FnMut::call_mut(&mut x, ())`. This is achieved by just autoderef'ing one time when calling something whose type is `&F` or `&mut F`.
2. Making the infinite recursion test in trait matching a bit more tailored. This involves adding a notion of "matching" types that looks to see if types are potentially unifiable (it's an approximation).

The PR also includes various small refactorings to the inference code that are aimed at moving the unification and other code into a library (I've got that particular change in a branch, these changes just lead the way there by removing unnecessary dependencies between the compiler and the more general unification code). 

Note that per rust-lang/rfcs#1023, adding impls like these would be a breaking change in the future. 

cc @japaric
cc @alexcrichton 
cc @aturon 

Fixes rust-lang#23015.
@alexcrichton alexcrichton merged commit 11111bb into rust-lang:master Apr 2, 2015
@bors
Copy link
Contributor

bors commented Apr 2, 2015

☔ The latest upstream changes (presumably #23955) made this pull request unmergeable. Please resolve the merge conflicts.

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.

FnMut not implemented for mutable FnMut implementors
5 participants