marker_traits: require EvaluatedToOk during winnowing#88139
Merged
Conversation
rust-highfive
added
the
S-waiting-on-review
Contributor
|
@bors r+ |
Collaborator
|
📌 Commit 3329f67 has been approved by |
bors
added
S-waiting-on-bors
Collaborator
Collaborator
|
☀️ Test successful - checks-actions |
6 tasks
traviscross
added a commit
to traviscross/rust
that referenced
this pull request
Mar 14, 2026
The `TypeOutlives` handler in `evaluate_predicate_recursively` checks whether a type in a `T: 'a` predicate has free regions, bound regions, or inference variables -- and if so, returns `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. The comment says "no free lifetimes or generic parameters", but the code checked `has_non_region_infer()` twice instead of checking `has_non_region_param()` for the second condition. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one overlapping impl beats another. With the bug, a `T: 'static`-bounded impl appeared equally strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes rust-lang#109481. This same symptom was originally filed as rust-lang#84917 and fixed in PR rust-lang#88139. Then PR rust-lang#102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Shortly after, rust-lang#109481 was filed. It noted the connection to rust-lang#102472 -- that it was only relevant after it -- but apparently no one noticed the duplicate condition.
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Jun 9, 2026
…ssing-param-check, r=lcnr Fix marker trait winnowing depending on impl order Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing `known-bug` test, which pointed me to rust-lang#109481. --- The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `has_non_region_param()`. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one impl beats another when there are multiple candidates. With the bug, a `T: 'static`-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes rust-lang#109481. This same symptom was originally filed as rust-lang#84917 and fixed in PR rust-lang#88139. Then PR rust-lang#102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Around this same time, rust-lang#109481 was filed. It noted the connection to rust-lang#102472 -- the bug only appeared after it -- but the duplicate condition was not noticed. r? @lcnr cc @oli-obk @nikomatsakis
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Jun 9, 2026
…ssing-param-check, r=lcnr Fix marker trait winnowing depending on impl order Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing `known-bug` test, which pointed me to rust-lang#109481. --- The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `has_non_region_param()`. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one impl beats another when there are multiple candidates. With the bug, a `T: 'static`-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes rust-lang#109481. This same symptom was originally filed as rust-lang#84917 and fixed in PR rust-lang#88139. Then PR rust-lang#102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Around this same time, rust-lang#109481 was filed. It noted the connection to rust-lang#102472 -- the bug only appeared after it -- but the duplicate condition was not noticed. r? @lcnr cc @oli-obk @nikomatsakis
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Jun 9, 2026
…ssing-param-check, r=lcnr Fix marker trait winnowing depending on impl order Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing `known-bug` test, which pointed me to rust-lang#109481. --- The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `has_non_region_param()`. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one impl beats another when there are multiple candidates. With the bug, a `T: 'static`-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes rust-lang#109481. This same symptom was originally filed as rust-lang#84917 and fixed in PR rust-lang#88139. Then PR rust-lang#102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Around this same time, rust-lang#109481 was filed. It noted the connection to rust-lang#102472 -- the bug only appeared after it -- but the duplicate condition was not noticed. r? @lcnr cc @oli-obk @nikomatsakis
rust-timer
added a commit
that referenced
this pull request
Jun 10, 2026
Rollup merge of #153847 - traviscross:TC/fix-typeoutlives-missing-param-check, r=lcnr Fix marker trait winnowing depending on impl order Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing `known-bug` test, which pointed me to #109481. --- The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `has_non_region_param()`. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one impl beats another when there are multiple candidates. With the bug, a `T: 'static`-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes #109481. This same symptom was originally filed as #84917 and fixed in PR #88139. Then PR #102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Around this same time, #109481 was filed. It noted the connection to #102472 -- the bug only appeared after it -- but the duplicate condition was not noticed. r? @lcnr cc @oli-obk @nikomatsakis
pull Bot
pushed a commit
to Kokoro2336/rust-analyzer
that referenced
this pull request
Jun 11, 2026
…am-check, r=lcnr Fix marker trait winnowing depending on impl order Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing `known-bug` test, which pointed me to rust-lang/rust#109481. --- The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `has_non_region_param()`. This meant that `TypeOutlives(T, 'static)` where `T` is a type parameter returned `EvaluatedToOk` -- claiming the result holds unconditionally -- when the correct answer is `EvaluatedToOkModuloRegions`. The distinction matters during marker trait winnowing in `prefer_lhs_over_victim`, which uses `must_apply_considering_regions()` (true only for `EvaluatedToOk`) to decide whether one impl beats another when there are multiple candidates. With the bug, a `T: 'static`-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one. Fixes rust-lang/rust#109481. This same symptom was originally filed as rust-lang/rust#84917 and fixed in PR rust-lang/rust#88139. Then PR rust-lang/rust#102472 rewrote the `TypeOutlives` handler, introducing the duplicate `has_non_region_infer()` and losing the param check, regressing this. Around this same time, rust-lang/rust#109481 was filed. It noted the connection to rust-lang/rust#102472 -- the bug only appeared after it -- but the duplicate condition was not noticed. r? @lcnr cc @oli-obk @nikomatsakis
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #84955, while it doesn't really fix it in a way that makes me happy it should prevent the issue for now and this
test can't be reproduced anyways, so it doesn't make much sense to keep it open.
fixes #84917 as only one of the impls depends on regions, so we now drop the ambiguous one instead of the correct one.
cc https://rust-lang.zulipchat.com/#narrow/stream/144729-wg-traits/topic/winnowing.20soundly/near/247899832
r? @nikomatsakis