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

Update orphan and overlap rules for RFC 1023 #23867

Merged
merged 6 commits into from
Apr 2, 2015

Conversation

nikomatsakis
Copy link
Contributor

This PR implements rust-lang/rfcs#1023. In the process it fixes #23086 and #23516. A few impls in libcore had to be updated, but the impact is generally pretty minimal. Most of the fallout is in the tests that probed the limits of today's coherence.

I tested and we were able to build the most popular crates along with iron (modulo errors around errors being sendable).

Fixes #23918.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@nikomatsakis
Copy link
Contributor Author

The integration of coherence checking into trait matching is a bit less clean than I would like -- it preserves the existing "intercrate mode". I'd like to rework select to remove that mode and instead consider that an impl like A: Foo without an associated where clause be considered ambiguous.

@nikomatsakis
Copy link
Contributor Author

Ah, and I just remembered a potential bug around subtyping in the way that I implemented the orphan check. I'll patch that. But it shouldn't really affect much, so I'll leave the PR open to use for reference while considering rust-lang/rfcs#1023.

@nikomatsakis
Copy link
Contributor Author

r? @pnkfelix

Since we will remove variance for trait matching, that complication is resolved the easy way. My concern was that the overlap checking wasn't checking all 4 combinations that seem to be necessary with variance (that is, instantiate impl A with skolemized parameters, then check whether A <: B or B <: A, and vice versa). But with invariance you need only check whether A = B, which is reflexive.

@nikomatsakis nikomatsakis changed the title [WIP] Update orphan and overlap rules for RFC 1023 Update orphan and overlap rules for RFC 1023 Apr 1, 2015
@nikomatsakis nikomatsakis force-pushed the issue-23086-take-3 branch 2 times, most recently from 6f214c6 to 1451d80 Compare April 1, 2015 12:11
@pnkfelix
Copy link
Member

pnkfelix commented Apr 1, 2015

Okay, I left some notes, but its all truly minor nits that I only recommend you do if you already are rebasing this PR for some other reason.

So r+ from me.

@pnkfelix
Copy link
Member

pnkfelix commented Apr 1, 2015

@bors r+ 6ba9692

@nikomatsakis
Copy link
Contributor Author

Addressed nits.

@nikomatsakis
Copy link
Contributor Author

@bors r=pnkfelix 67abce2

@bors
Copy link
Contributor

bors commented Apr 1, 2015

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

local only if matches `FUNDAMENTAL(LocalType)`, where `FUNDAMENTAL`
includes `&T` and types marked as fundamental (which includes `Box`).
Also apply these tests to negative reasoning.
`Fn` traits are considered fundamental, along with `Box` (though that is
mostly for show; the real type is `~T` in the compiler).
since `Option` is not fundamental and hence the old impls run afoul of
the orphan rules.
@nikomatsakis
Copy link
Contributor Author

@bors r=pnkfelix 15b58fe

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 1, 2015
…pnkfelix

This PR implements rust-lang/rfcs#1023. In the process it fixes rust-lang#23086 and rust-lang#23516. A few impls in libcore had to be updated, but the impact is generally pretty minimal. Most of the fallout is in the tests that probed the limits of today's coherence.

I tested and we were able to build the most popular crates along with iron (modulo errors around errors being sendable).

Fixes rust-lang#23918.
sidesteps a coherence difficulty where `liballoc` had to prove that
`&str: !Error`, which didn't involve any local types.
@nikomatsakis
Copy link
Contributor Author

@bors r=aturon 19d3dab

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 1, 2015
This PR implements rust-lang/rfcs#1023. In the process it fixes rust-lang#23086 and rust-lang#23516. A few impls in libcore had to be updated, but the impact is generally pretty minimal. Most of the fallout is in the tests that probed the limits of today's coherence.

I tested and we were able to build the most popular crates along with iron (modulo errors around errors being sendable).

Fixes rust-lang#23918.
@bors
Copy link
Contributor

bors commented Apr 2, 2015

⌛ Testing commit 19d3dab with merge 2b42de0...

@alexcrichton alexcrichton merged commit 19d3dab into rust-lang:master Apr 2, 2015
@bluss
Copy link
Member

bluss commented Apr 2, 2015

This should have been marked breaking-change?

@tbu-
Copy link
Contributor

tbu- commented Apr 3, 2015

Yup. Broke something for rust-ascii.

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.

Tracking issue for Rebalancing Coherence (RFC 1023) Forward compatibility hazard from coherence rules
7 participants