-Znext-solver Remove a problematic assertion from probing object bound candidates#152859
Open
ShoyuVanilla wants to merge 1 commit intorust-lang:mainfrom
Open
-Znext-solver Remove a problematic assertion from probing object bound candidates#152859ShoyuVanilla wants to merge 1 commit intorust-lang:mainfrom
-Znext-solver Remove a problematic assertion from probing object bound candidates#152859ShoyuVanilla wants to merge 1 commit intorust-lang:mainfrom
Conversation
f07494b to
4a25656
Compare
4a25656 to
0f257f0
Compare
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
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.
Fixes #152789 and fixes #151329
r? lcnr
Yeah, this is the worst fix for such kind of ICEs. But wait, let me make some excuses 😅
Root Cause
From the above code, when we try ty prove the well-formedness of
<dyn Trait<i32, Assoc = i64> as Trait<U::FooAssoc>>::Assoc, we probe the object bound candidates.rust/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs
Lines 884 to 904 in 41198cb
We gather the object bounds from the above lines: we have a kv pair
(Trait::Assoc, vec![<dyn Trait<i32, Assoc = i64> as Trait<i32>>::Assoc = i64])formapping.And then try to eagerly replace the projection types in our goal =
<dyn Trait<i32, Assoc = i64> as Trait<U::FooAssoc>>::Assoc = ?_, i.e.<dyn Trait<i32, Assoc = i64> as Trait<U::FooAssoc>>::Assoc.rust/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs
Lines 942 to 967 in 41198cb
We lookup
mappingby the assoc ty id:Trait::Assoc, so we getreplacements = vec![<dyn Trait<i32, Assoc = i64> as Trait<i32>>::Assoc = i64].But
dyn Trait<i32, Assoc = i64> as Trait<i32>>::Assoc = i64cannot match with<dyn Trait<i32, Assoc = i64> as Trait<U::FooAssoc>>::Assoc, panic💥Actually, that goal should be normalized/proved by the where clause from the param env:
dyn Trait<i32, Assoc = i64>: Trait<U::FooAssoc>.But unfortunately, that has the same assoc ty id with the object's bound so we trigger the assertion, which has no problem when we have different assoc ty ids, instead of giving up the normalization.
In short, the
replacementsshould beNoneand thus we should short-circuit, but unfortunately we have some confusing one by the same assoc ty id.Isn't there any alternative?
Well, perhaps we can gather trait predicates from the param env as well into the
mappingsbut this doesn't feel correct because..dyn Trait<i32, Assoc = i64>: Trait2<U::FooAssoc>anddyn Trait<i32, Assoc = i64>: Trait<U::FooAssoc>is not so structural.Or perhaps we index
mappingsby(assoc_ty, args), so don't end up gettingreplacements = Some(_)when it should beNone.But there might be some cases that the args are not necessarily equal but can be equalized modulo
Relateand if so, we have to just try match for all keys with the same assoc ty id, which doesn't make the situation any betterConclusion
So, I think the simplest fix is just removing the possibly wrong assertion. Yeah, this feels lazy but I have some reasons 😅
After all, the goal can be proven, not with the object bound candidates but with the param env and that's the right path.