Allow specifying multiple bounds for same associated item (remove E0719) #143146
+598
−1,136
Conversation
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
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in diagnostic error codes HIR ty lowering was modified cc @fmease |
rustbot
added
needs-fcp
This comment has been minimized.
This comment has been minimized.
|
more importantly cc @rust-lang/types rather than T-lang |
fmease
added
T-types
fmease
changed the title
fmease
reviewed
Jun 28, 2025
fmease
changed the title
Jules-Bertholet
changed the title
fmease
changed the title
Jules-Bertholet
force-pushed
the
remove-e0719
branch
from
b4a13f9 to
66ab8f8
Compare
This comment has been minimized.
This comment has been minimized.
fmease
reviewed
Jun 28, 2025
There was a problem hiding this comment.
(commenting on a random file so it's threaded)
Thanks for incorporating my suggestion about adding more tests!
What are the interactions with trait object types? It's possible that lower_trait_object_ty still ends up rejecting such dyn-Traits (e.g., dyn Trait<X = i32, X = u64>) due to its intricate and delicate logic but I might not be right. For example, we currently reject:
#![feature(trait_alias)]
trait Iter = Iterator<Item = ()>;
fn f(_: Box<dyn Iter<Item = u64>>) {} //~ ERROR conflicting associated type bounds for `Item` when expanding trait aliasI'm curious about whether that gets triggered for sth. like dyn Trait<X = i32, X = u64> 🤔 If not, I wonder if we should then also lift this trait alias restriction?
I don't know the soundness implications of allowing these equality predicates in trait object types since I haven't spent much thought on it. In my mind, a trait object type with "impossible" existential projection predicates should be fine, it would just be impossible to construct a value of it. Anyhow, I'm not qualified enough to draw a proper conclusion.
There was a problem hiding this comment.
Good catch! It looks like we do trigger that error. I added tests, and tweaked the error message to be more generic.
Interestingly, it seems that if you do:
trait Sup {
type Assoc;
}
trait Sub: Sup<Assoc = u32> {}
type Foo = dyn Sub<Assoc = i32>;Then no error is emitted, but Foo does not implement Sub.
|
r? fmease |
|
|
|
☔ The latest upstream changes (presumably #144145) made this pull request unmergeable. Please resolve the merge conflicts. |
Jules-Bertholet
force-pushed
the
remove-e0719
branch
2 times, most recently
from
d120d7d to
c9410f5
Compare
fmease
reviewed
Sep 8, 2025
| fn foo() {} | ||
| } | ||
|
|
||
| trait Subtrait: Trait<Gat<u32> = u32, Gat<u64> = u64> {} |
There was a problem hiding this comment.
(minor) the subdirectory associated-type-bounds/ is meant for ATBs only as in TraitRef<AssocTy: Bounds> the recently stabilized feature, so not for assoc item bindings / equality constraints.
|
rerolling back to a types reviewer since I don't know t-types's vibe and can't initiate an fcp (whether close or merge) r? types |
This comment has been minimized.
This comment has been minimized.
This hard error serves no purpose. Fixes rust-lang#143143
Jules-Bertholet
force-pushed
the
remove-e0719
branch
from
54092be to
1a67700
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
r? BoxyUwU I'll get to this before the end of the week |
|
I actually imagine this is just a lang FCP. I don't see foresee any issues or complications on the types side. |
fmease
added
I-lang-nominated
traviscross
added
the
P-lang-drag-1
|
@Jules-Bertholet, if you could write up what's being proposed for stabilization for the lang nomination, particularly regarding what the edges of this are, the interaction with I don't see the restriction this relaxes in the Reference, so if for whatever reason we decide not to do this, we should document the restriction. cc @rust-lang/lang-docs |
|
The following code ICEs with this PR: pub trait Trait {
type Assoc: Copy + 'static;
}
const fn conjure<T>() -> T {
panic!()
}
const fn get_assoc<T: Trait>() -> impl Copy {
conjure::<<T as Trait>::Assoc>()
}
pub fn foo<T: Trait<Assoc = i32, Assoc = i64>>() {
const {
get_assoc::<T>();
}
}Compiler output |
It’s a pre-existing issue, this PR only makes it very slightly easier to trigger: #146371 |
|
This PR proposes to remove E0719 from the compiler, accepting code that used to be forbidden by it. E0719 forbids constraining the same associated item twice within the same angle-bracket delimited associated item bound list (the
With this PR, all those previously forbidden examples would start working, as well as their APIT and RPIT equivalents. Types like N.B., the following works today: trait Sup {
type Assoc;
}
trait Sub: Sup<Assoc = u32> {}
// `Foo: Sub` doesn't hold
type Foo = dyn Sub<Assoc = i32>;Arguably, this is inconsistent, and we might want to change it in the future—either by rejecting the snippet, or by accepting AlternativesWe could try to adopt a more complex rule that accepts only the truly useful cases (like the first entry in the table above) while rejecting everything else. However, I don’t see the point of adding all that extra complexity to the compiler and language specification. The extra permissiveness could also be useful for macros. |
|
Makes sense to me. This change to the language has the virtue of making the Reference more correct. @rfcbot fcp merge |
|
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
rust-rfcbot
added
proposed-final-comment-period
|
Given that From a UX perspective I would expect that The behavior with trait objects is a bit more interesting on the T-types side and I kind of would like to keep erroring for them.
This seems... subtle and somewhat undesirable. I do think T-types should be involved there. You've got the following test assert_eq!(
TypeId::of::<dyn Iterator<Item = u32>>(),
TypeId::of::<dyn Iterator<Item = u32, Item = u32>>()
);What happens for We no longer merge duplicate projection bounds for trait objects after lowering since #136458, so if we fail to merge them during HIR type lowering, If |
I agree with this. I don't think it needs to block necessarily. +1 for the overall idea, modulo the concern about dyn. @rfcbot reviewed |
It seems that these end up being rejected by the same check that rejects |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This hard error serves no purpose.
Fixes #143143.
(Documentation of E0719: https://doc.rust-lang.org/stable/error_codes/E0719.html)
@rustbot label T-lang needs-fcp