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

Use is_global in candidate_should_be_dropped_in_favor_of #90375

Merged
merged 3 commits into from
Oct 30, 2021

Conversation

yanok
Copy link
Contributor

@yanok yanok commented Oct 28, 2021

This manifistated in #90195 with compiler being unable to keep
one candidate for a trait impl, if where is a global impl and more
than one trait bound in the where clause.

Before #87280 candidate_should_be_dropped_in_favor_of was using
TypeFoldable::is_global() that was enough to discard the two
ParamCandidates. But #87280 changed it to use
TypeFoldable::is_known_global() instead, which is pessimistic, so
now the compiler drops the global impl instead (because
is_known_global is not sure) and then can't decide between the
two ParamCandidates.

Switching it to use is_global again solves the issue.

Fixes #90195.

This manifistated in rust-lang#90195 with compiler being unable to keep
one candidate for a trait impl, if where is a global impl and more
than one trait bound in the where clause.

Before rust-lang#87280 `candidate_should_be_dropped_in_favor_of` was using
`TypeFoldable::is_global()` that was enough to discard the two
`ParamCandidate`s. But rust-lang#87280 changed it to use
`TypeFoldable::is_known_global()` instead, which is pessimistic, so
now the compiler drops the global impl instead (because
`is_known_global` is not sure) and then can't decide between the
two `ParamCandidate`s.

Switching it to use `is_global` again solves the issue.

Fixes rust-lang#90195.
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 28, 2021
@rust-log-analyzer

This comment has been minimized.

@yanok
Copy link
Contributor Author

yanok commented Oct 29, 2021

r? @lcnr

Hey, since you introduced is_known_global, could you please review this? Does c6b6901 fix it once and for all?

From is_known_global description it looks like we might want to be certain and use is_global instead.

@lcnr
Copy link
Contributor

lcnr commented Oct 29, 2021

Does c6b6901 fix it once and for all?

#90266 does fix that issue in a more general way, but the underlying issue with is_known_global still exists. Having the "definitely" vs "potentially" split was the best way we knew to work towards a necessary change for feature(generic_const_exprs), but it does worsen the experience when working with types in rustc so I am currently considering alternatives.

Your PR is correct and probably just generally a good idea, so
@bors r+

@bors
Copy link
Contributor

bors commented Oct 29, 2021

📌 Commit 9a0a622 has been approved by lcnr

@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 Oct 29, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 30, 2021
…laumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#90156 (Remove underlines from non-top docblocks.)
 - rust-lang#90183 (Show all Deref implementations recursively)
 - rust-lang#90202 (Improve and test cross-crate hygiene)
 - rust-lang#90375 (Use `is_global` in `candidate_should_be_dropped_in_favor_of`)
 - rust-lang#90399 (Skipping verbose diagnostic suggestions when calling .as_ref() on type not implementing AsRef)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 06bb1ff into rust-lang:master Oct 30, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 30, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rkyv in Rust 1.56: type annotations needed
7 participants