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

Permit perfect forwarding for Fn traits #19884

Merged
merged 24 commits into from
Dec 19, 2014

Conversation

nikomatsakis
Copy link
Contributor

Rewrite how the HRTB algorithm matches impls against obligations. Instead of impls providing higher-ranked trait-references, impls now once again only have early-bound regions. The skolemization checks are thus moved out into trait matching itself. This allows to implement "perfect forwarding" impls like those described in #19730. This PR builds on a previous PR that was already reviewed by @pnkfelix.

r? @pnkfelix

Fixes #19730

@japaric
Copy link
Member

japaric commented Dec 15, 2014

@nikomatsakis There are some make tidy issues.

@nikomatsakis
Copy link
Contributor Author

Just pushed two more commits that (I think) fix the lingering compile-fail tests, though my local build isn't 100% complete yet. I may revise the "WIP" commits, though I'll try to leave the others so as not to interfere with the review.

@japaric
Copy link
Member

japaric commented Dec 16, 2014

@nikomatsakis And this needs a rebase :-)

@nikomatsakis nikomatsakis force-pushed the issue-19730-perfect-forwarding branch from 27cf881 to 649eab7 Compare December 16, 2014 16:19
want_hrtb::<AnyInt>()
}

// StaticInt only implements Foo<&'a int> for 'a, so it is an error.
Copy link
Member

Choose a reason for hiding this comment

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

As written it is not clear that the 'a being referred to is implicitly bound to one particular lifetime, namely 'static. This should be rewritten as "only implements Foo<&'a int> where 'a == 'static". Or "only implements Foo<&'static int>` if you prefer.

@pnkfelix
Copy link
Member

Okay, from what I can tell, this all seems to hang together. All of my notes above are largely nits/typos about comments.

r=me after you rebase.

@nikomatsakis nikomatsakis force-pushed the issue-19730-perfect-forwarding branch 4 times, most recently from 30dc69e to 2e0cd29 Compare December 19, 2014 03:47
@nikomatsakis
Copy link
Contributor Author

Giving high priority because this is directly blocking progress on both unboxed closures and associated types.

have such a silly over-engineered interface.
impl for fn pointer types including bound regions, unfortunately.
issue-14039 test in a more natural way. Previously the "type we will
cast to" was hidden unless it was an integer.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ions bound in the impl before searching for `Copy` implements for all fields, leading to problems in the "copyability check". Basically the copyability check would wind up looking for an impl of `for<'tcx> Foo<&'tcx T>`. The impl that exists however is `impl<T> Copy for Foo<T>` and the current rules do not consider that a match (something I would like to revise in a later PR).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
… TraitRef.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…han "skolemize" -- strictly speaking, this is not skolemization, because it is not discharging quantifiers. Also, the trait selection code will still be doing true skolemization, so it would be a confusing overlap of names.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…than having FnSig carry an implicit binding level. This means that we be more typesafe in general, since things that instantiate bound regions can drop the Binder to reflect that.
read (`//!` is intrusive) and annoying to edit (must maintain a prefix
on every line). Since the only purpose of a `doc.rs` file is to have a
bunch of text, using `/*!` and `*/` without indentations seems
appropriate.
lifetimes. This currently causes an ICE; it should (ideally) work, but
failing that at least give a structured error. For the purposes of
this PR, though, workaround is fine.
though it includes a `fn()`. This is really a more general
problem but I wanted to ensures that `bytes` in particular
remains working due to rust-lang#12677.
not dug deeply into what is going on here, although the errors ARE
somewhat surprising.
@nikomatsakis nikomatsakis force-pushed the issue-19730-perfect-forwarding branch from 2e0cd29 to ebf1e4f Compare December 19, 2014 08:30
bors added a commit that referenced this pull request Dec 19, 2014
…ng, r=pnkfelix

Rewrite how the HRTB algorithm matches impls against obligations. Instead of impls providing higher-ranked trait-references, impls now once again only have early-bound regions. The skolemization checks are thus moved out into trait matching itself. This allows to implement "perfect forwarding" impls like those described in #19730. This PR builds on a previous PR that was already reviewed by @pnkfelix.

r? @pnkfelix 

Fixes #19730
@bors bors merged commit ebf1e4f into rust-lang:master Dec 19, 2014
@nikomatsakis nikomatsakis deleted the issue-19730-perfect-forwarding branch March 30, 2016 16:14
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.

HRTB rules do not permit "perfect forwarding" of the Fn traits
4 participants