Don't discard marker trait impls when inference variables are present #68057
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes rust-lang#61651 Previously, we would unconditionally discard impl candidates for marker traits during trait selection. However, if the predicate had inference variables, this could have the effect of constrainting inference variables (due to a successful trait selection) when we would have otherwise failed due to mutliple applicable impls, This commit prevents marker trait impls from being discarded while the obligation predicate has any inference variables, ensuring that discarding impls will never cause us to incorrectly constraint inference variables.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 4840cd8 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Jan 20, 2020
…wjasper Don't discard marker trait impls when inference variables are present Fixes #61651 Previously, we would unconditionally discard impl candidates for marker traits during trait selection. However, if the predicate had inference variables, this could have the effect of constrainting inference variables (due to a successful trait selection) when we would have otherwise failed due to mutliple applicable impls, This commit prevents marker trait impls from being discarded while the obligation predicate has any inference variables, ensuring that discarding impls will never cause us to incorrectly constraint inference variables.
|
☀️ Test successful - checks-azure |
Aaron1011
added a commit
to Aaron1011/rust
that referenced
this pull request
Feb 4, 2020
See rust-lang#29864 This has been replaced by `#[feature(marker_trait_attr)]` A few notes: * Due to PR rust-lang#68057 not yet being in the bootstrap compiler, it's necessary to continue using `#![feature(overlapping_marker_traits)]` under `#[cfg(bootstrap)]` to work around type inference issues. * I've updated tests that used `overlapping_marker_traits` to now use `marker_trait_attr` where applicable The test `src/test/ui/overlap-marker-trait.rs` doesn't make any sense now that `overlapping_marker_traits`, so I removed it. The test `src/test/ui/traits/overlap-permitted-for-marker-traits-neg.rs` now fails, since it's no longer possible to have multiple overlapping negative impls of `Send`. I believe that this is the behavior we want (assuming that `Send` is not going to become a `#[marker]` trait, so I renamed the test to `overlap-permitted-for-marker-traits-neg`
bors
added a commit
that referenced
this pull request
Feb 4, 2020
Remove the `overlapping_marker_traits` feature See #29864 This has been replaced by `#[feature(marker_trait_attr)]` A few notes: * Due to PR #68057 not yet being in the bootstrap compiler, it's necessary to continue using `#![feature(overlapping_marker_traits)]` under `#[cfg(bootstrap)]` to work around type inference issues. * I've updated tests that used `overlapping_marker_traits` to now use `marker_trait_attr` where applicable The test `src/test/ui/overlap-marker-trait.rs` doesn't make any sense now that `overlapping_marker_traits`, so I removed it. The test `src/test/ui/traits/overlap-permitted-for-marker-traits-neg.rs` now fails, since it's no longer possible to have multiple overlapping negative impls of `Send`. I believe that this is the behavior we want (assuming that `Send` is not going to become a `#[marker]` trait, so I renamed the test to `overlap-permitted-for-marker-traits-neg`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #61651
Previously, we would unconditionally discard impl candidates for marker
traits during trait selection. However, if the predicate had inference
variables, this could have the effect of constrainting inference
variables (due to a successful trait selection) when we would have
otherwise failed due to mutliple applicable impls,
This commit prevents marker trait impls from being discarded while the
obligation predicate has any inference variables, ensuring that
discarding impls will never cause us to incorrectly constraint inference
variables.