-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Re-enable trivial bounds #51042
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Re-enable trivial bounds #51042
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
b66c17e
to
3339eea
Compare
Pushed a commit to fix #51044 as well |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
3339eea
to
b384d6e
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
b384d6e
to
ffb93bf
Compare
Triage ping, @nikomatsakis, this PR is waiting for your review. |
Ping from triage! This PR needs a review, can @nikomatsakis or someone else from @rust-lang/compiler review this? |
I will review this one, but I haven't gotten to it yet. |
@@ -2025,13 +2022,46 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { | |||
other: &EvaluatedCandidate<'tcx>) | |||
-> bool | |||
{ | |||
// Check if a bound would previously have been removed when normalizing | |||
// the param_env so that it can be given the lowest priority. See | |||
// #50825 for the motivation for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh. But makes sense. Chalk can't come soon enough!
src/librustc/traits/select.rs
Outdated
let tcx = self.tcx().global_tcx(); | ||
return tcx.specializes((other_def, victim_def)) || | ||
tcx.impls_are_allowed_to_overlap(other_def, victim_def); | ||
// FIXME also have an is_global here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what this FIXME is suggesting -- you did add one in ParamCandidate
case, which seems like the case where it would be relevant..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just forgot to remove the comment
I confirm that this makes diesel master compile again 🎉 |
d748578
to
c8f4ef1
Compare
Removes extra global bounds at the winnowing stage rather than when normalizing the param_env. This avoids breaking inference when there is a global bound.
c8f4ef1
to
a1bddcf
Compare
@bors r+ |
📌 Commit a1bddcf has been approved by |
🌲 The tree is currently closed for pull requests below priority 1, this pull request will be tested once the tree is reopened |
@bors r- -- oops, had a few questions |
In particular, I wanted to know what was up with that FIXME -- maybe we should just remove it? I suspect it is outdated. |
Oh, I missed that =) @bors r+ p=1 -- fixes regressions |
📌 Commit a1bddcf has been approved by |
…atsakis Re-enable trivial bounds cc #50825 Remove implementations from global bounds in winnowing when there is ambiguity. This results in the reverse of #24066 happening sometimes. I'm not sure if anything can be done about that though. cc #48214 r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
cc #50825
Remove implementations from global bounds in winnowing when there is ambiguity.
This results in the reverse of #24066 happening sometimes. I'm not sure if anything can be done about that though.
cc #48214
r? @nikomatsakis