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

Tweak E0599 and elaborate_predicates #106788

Merged
merged 4 commits into from
Jan 14, 2023
Merged

Conversation

estebank
Copy link
Contributor

CC #86377.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2023

r? @petrochenkov

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 12, 2023
@estebank estebank marked this pull request as ready for review January 12, 2023 21:49
@estebank
Copy link
Contributor Author

r? @compiler-errors

Comment on lines +1597 to +1598
if o.predicate.to_opt_poly_trait_pred().map(|p| p.def_id())
== self.tcx.lang_items().sized_trait()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if o.predicate.to_opt_poly_trait_pred().map(|p| p.def_id())
== self.tcx.lang_items().sized_trait()
if let Some(trait_pred) = o.predicate.to_opt_poly_trait_pred()
&& self.tcx.lang_items().sized_trait() == Some(trait_pred.def_id())

This will never happen in practice, but it's better to deal with the None == None case when lang-items are not defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the None == None case you envision? Because trait_pred might have been None (so we don't get into this logic) and sized_trait() might be None which might cause a build where the sized trait lang item isn't present to be too verbose (with extra labels), no other behavioral change.

@@ -3,7 +3,7 @@
use std::collections::HashSet;
use std::hash::Hash;

fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash {
fn is_subset<T>(this: &HashSet<T>, other: &HashSet<T>) -> bool where T: Eq, T: Hash, T: PartialEq {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I don't consider this an improvement -- T: Eq implies T: PartialEq by elaboration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, so I tested hiding the suggestions when a parent obligation was present, but that broke some existing tests with applicable suggestions. I'll see if we can inspect the obligation cause code instead to avoid these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@compiler-errors
Copy link
Member

Ideally we'd distinguish elaborated predicates and "root" predicates so that we only pass the latter for suggestions, but I guess whatever 🤔

r=me

@estebank
Copy link
Contributor Author

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Jan 13, 2023

📌 Commit 22a0e4f has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jan 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 14, 2023
…ompiler-errors

Tweak E0599 and elaborate_predicates

CC rust-lang#86377.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 14, 2023
…ompiler-errors

Tweak E0599 and elaborate_predicates

CC rust-lang#86377.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#106046 (Fix mir-opt tests for big-endian platforms)
 - rust-lang#106470 (tidy: Don't include wasm32 in compiler dependency check)
 - rust-lang#106566 (Emit a single error for contiguous sequences of unknown tokens)
 - rust-lang#106644 (Update the wasi-libc used for the wasm32-wasi target)
 - rust-lang#106665 (Add note when `FnPtr` vs. `FnDef` impl trait)
 - rust-lang#106752 (Emit a hint for bad call return types due to generic arguments)
 - rust-lang#106788 (Tweak E0599 and elaborate_predicates)
 - rust-lang#106831 (Use GitHub yaml templates for ICE, Docs and Diagnostics tickets)
 - rust-lang#106846 (Improve some comments and names in parser)
 - rust-lang#106848 (Fix wrong path in triage bot autolabel for wg-trait-solver-refactor)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8c538f7 into rust-lang:master Jan 14, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 14, 2023
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants