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

Don't create impl candidates when obligation contains errors #73005

Merged
merged 3 commits into from
Jun 11, 2020

Conversation

Aaron1011
Copy link
Member

Fixes #72839

In PR #72621, trait selection was modified to no longer bail out early
when an error type was encountered. This allowed us treat ty::Error as
Sized, causing us to avoid emitting a spurious "not sized" error after
a type error had already occured.

However, this means that we may now try to match an impl candidate
against the error type. Since the error type will unify with almost
anything, this can cause us to infinitely recurse (eventually triggering
an overflow) when trying to verify certain where clauses.

This commit causes us to skip generating any impl candidates when an
error type is involved.

Fixes rust-lang#72839

In PR rust-lang#72621, trait selection was modified to no longer bail out early
when an error type was encountered. This allowed us treat `ty::Error` as
`Sized`, causing us to avoid emitting a spurious "not sized" error after
a type error had already occured.

However, this means that we may now try to match an impl candidate
against the error type. Since the error type will unify with almost
anything, this can cause us to infinitely recurse (eventually triggering
an overflow) when trying to verify certain `where` clauses.

This commit causes us to skip generating any impl candidates when an
error type is involved.
@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 Jun 4, 2020
@estebank
Copy link
Contributor

estebank commented Jun 5, 2020

You'll need to rebless.

// emitting additional spurious errors, since we're guaranteed
// to have emitted at least one.
if stack.obligation.references_error() {
debug!("no results for error type, treating as ambiguous");
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a delay_span_bug, just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

With #70551, seeing an error type guarantees that delay_span_bug has been called.

Copy link
Contributor

Choose a reason for hiding this comment

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

r=me when CI is green then :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you want to wait for #70551, or can I r+ this now?

Co-authored-by: Randy Taylor <tehgecKozzz@gmail.com>
@Aaron1011
Copy link
Member Author

The MCP for #70551 was accepted, so I'm going to

@bors r=estebank

@bors
Copy link
Contributor

bors commented Jun 9, 2020

📌 Commit ae42c91 has been approved by estebank

@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 Jun 9, 2020
@Aaron1011 Aaron1011 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 9, 2020
@Aaron1011
Copy link
Member Author

Nominating for beta backport: while #72839 is 'only' a diagnostics issue, it's a very annoying one. When working on rustc, I'm frequently getting thousands of lines of useless output when my change fails to compile.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 10, 2020
Rollup of 9 pull requests

Successful merges:

 - rust-lang#72706 (Add windows group to triagebot)
 - rust-lang#72789 (resolve: Do not suggest imports from the same module in which we are resolving)
 - rust-lang#72890 (improper ctypes: normalize return types and transparent structs)
 - rust-lang#72897 (normalize adt fields during structural match checking)
 - rust-lang#73005 (Don't create impl candidates when obligation contains errors)
 - rust-lang#73023 (Remove noisy suggestion of hash_map )
 - rust-lang#73070 (Add regression test for const generic ICE in rust-lang#72819)
 - rust-lang#73157 (Don't lose empty `where` clause when pretty-printing)
 - rust-lang#73184 (Reoder order in which MinGW libs are linked to fix recent breakage)

Failed merges:

r? @ghost
@bors bors merged commit 024f025 into rust-lang:master Jun 11, 2020
@spastorino spastorino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 17, 2020
@pnkfelix
Copy link
Member

unilaterally accepting for beta-backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 17, 2020
@Mark-Simulacrum
Copy link
Member

@Aaron1011 -- on beta, it looks like src/librustc_trait_selection/traits/select/candidate_assembly.rs doesn't exist. Can you prepare a commit that backports this cleanly? (Or alternatively we can just not backport this as it's solely a diagnostics fix I guess?)

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Jun 26, 2020
@Aaron1011
Copy link
Member Author

@Mark-Simulacrum: I've created e461d2e

Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this pull request Jun 26, 2020
@Mark-Simulacrum
Copy link
Member

Thanks, included.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2020
…ulacrum

[beta] backports

This PR backports the following:

* Make novel structural match violations not a `bug` rust-lang#73446
* linker: Never pass `-no-pie` to non-gnu linkers rust-lang#73384
* Disable the `SimplifyArmIdentity` pass rust-lang#73262
* Allow inference regions when relating consts rust-lang#73225
* Fix link error with #[thread_local] introduced by rust-lang#71192 rust-lang#73065
* Ensure stack when building MIR for matches rust-lang#72941
* Don't create impl candidates when obligation contains errors rust-lang#73005

r? @ghost
@Elinvynia Elinvynia removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 6, 2020
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2024
…ompiler-errors

 Don't Create `ParamCandidate` When Obligation Contains Errors

Fixes rust-lang#121941

I'm not sure if I understand this correctly but this bug was caused by an error type incorrectly matching against `ParamCandidate`. This was introduced by the changes made in rust-lang#72621 (figured using cargo-bisect-rustc).

This PR fixes it by skipping `ParamCandidate` generation when an error type is involved. Also, this is similar to rust-lang#73005 but addresses `ParamCandidate` instead of `ImplCandidate`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2024
…ompiler-errors

 Don't Create `ParamCandidate` When Obligation Contains Errors

Fixes rust-lang#121941

I'm not sure if I understand this correctly but this bug was caused by an error type incorrectly matching against `ParamCandidate`. This was introduced by the changes made in rust-lang#72621 (figured using cargo-bisect-rustc).

This PR fixes it by skipping `ParamCandidate` generation when an error type is involved. Also, this is similar to rust-lang#73005 but addresses `ParamCandidate` instead of `ImplCandidate`.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2024
Rollup merge of rust-lang#122360 - veera-sivarajan:bugfix-121941, r=compiler-errors

 Don't Create `ParamCandidate` When Obligation Contains Errors

Fixes rust-lang#121941

I'm not sure if I understand this correctly but this bug was caused by an error type incorrectly matching against `ParamCandidate`. This was introduced by the changes made in rust-lang#72621 (figured using cargo-bisect-rustc).

This PR fixes it by skipping `ParamCandidate` generation when an error type is involved. Also, this is similar to rust-lang#73005 but addresses `ParamCandidate` instead of `ImplCandidate`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.
Projects
None yet
10 participants