Skip to content

Check for negative impls when finding auto traits #55356

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

Merged
merged 3 commits into from
Nov 13, 2018

Conversation

Aaron1011
Copy link
Member

Fixes #55321

When AutoTraitFinder begins examining a type, it checks for an explicit
negative impl. However, it wasn't checking for negative impls found when
calling 'select' on predicates found from nested obligations.

This commit makes AutoTraitFinder check for negative impls whenever it
makes a call to 'select'. If a negative impl is found, it immediately
bails out.

Normal users of SelectioContext don't need to worry about this, since
they stop as soon as an Unimplemented error is encountered. However, we
add predicates to our ParamEnv when we encounter this error, so we need
to handle negative impls specially (so that we don't try adding them to
our ParamEnv).

Fixes rust-lang#55321

When AutoTraitFinder begins examining a type, it checks for an explicit
negative impl. However, it wasn't checking for negative impls found when
calling 'select' on predicates found from nested obligations.

This commit makes AutoTraitFinder check for negative impls whenever it
makes a call to 'select'. If a negative impl is found, it immediately
bails out.

Normal users of SelectioContext don't need to worry about this, since
they stop as soon as an Unimplemented error is encountered. However, we
add predicates to our ParamEnv when we encounter this error, so we need
to handle negative impls specially (so that we don't try adding them to
our ParamEnv).
@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 25, 2018
@TimNN
Copy link
Contributor

TimNN commented Nov 6, 2018

Ping from triage @nikomatsakis: This PR requires your review.

@QuietMisdreavus QuietMisdreavus added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 7, 2018
@QuietMisdreavus
Copy link
Member

Beta-nominating since the ICE this is fixing is on stable (and has been for a while).

Co-Authored-By: Aaron1011 <aa1ronham@gmail.com>
@nikomatsakis
Copy link
Contributor

Sorry for taking so long to review this. I see it's quite a small patch. To be honest, I'm still a bit nervous by the auto-trait finder code, so I kept putting off in order to do a more thorough "re-read" of that code, which is unfortunate. Anyway, seems fine for now.

@nikomatsakis
Copy link
Contributor

@bors r+ p=1

@bors
Copy link
Collaborator

bors commented Nov 13, 2018

📌 Commit 56acb2a has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2018
@nikomatsakis
Copy link
Contributor

Giving high priority because this is a regression and it's been sitting around a while.

@bors
Copy link
Collaborator

bors commented Nov 13, 2018

⌛ Testing commit 56acb2a with merge 9fefb67...

bors added a commit that referenced this pull request Nov 13, 2018
…sakis

Check for negative impls when finding auto traits

Fixes #55321

When AutoTraitFinder begins examining a type, it checks for an explicit
negative impl. However, it wasn't checking for negative impls found when
calling 'select' on predicates found from nested obligations.

This commit makes AutoTraitFinder check for negative impls whenever it
makes a call to 'select'. If a negative impl is found, it immediately
bails out.

Normal users of SelectioContext don't need to worry about this, since
they stop as soon as an Unimplemented error is encountered. However, we
add predicates to our ParamEnv when we encounter this error, so we need
to handle negative impls specially (so that we don't try adding them to
our ParamEnv).
@bors
Copy link
Collaborator

bors commented Nov 13, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 9fefb67 to master...

@bors bors merged commit 56acb2a into rust-lang:master Nov 13, 2018
@nagisa
Copy link
Member

nagisa commented Nov 15, 2018

We have decided to not backport this fix. It fixes a stable-to-stable regression that involves an unstable feature. Since it hasn’t been noticed for such a long time, it does not appear to be important enough to warrant a backport.

@nagisa nagisa removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants