-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Don't even try to combine consts with incompatible types #108947
Don't even try to combine consts with incompatible types #108947
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
oops forgor to r? r? @BoxyUwU |
This comment was marked as outdated.
This comment was marked as outdated.
// and unify it against impl #2's trait ref with fresh vars. The trait ref of | ||
// impl #1 is `S<Const(Param(N/#0): usize)>` because of astconv using the type | ||
// from the `L` substitution of struct `S`. This unifies successfully with impl #2, |
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.
The trait ref of impl #1 is
S<Const(Param(N/#0): usize)>
because of astconv using the type from theL
substitution of structS
.
I guess I could be a bit more explicit here -- I'm talking about this code:
rust/compiler/rustc_hir_analysis/src/astconv/mod.rs
Lines 449 to 458 in 39f2657
(GenericParamDefKind::Const { .. }, GenericArg::Const(ct)) => { | |
ty::Const::from_opt_const_arg_anon_const( | |
tcx, | |
ty::WithOptConstParam { | |
did: ct.value.def_id, | |
const_param_did: Some(param.def_id), | |
}, | |
) | |
.into() | |
} |
So when instantiating the substs for S<N>
, we use the const param did from L
, and end up with S<Const(Param(N/#0): usize)>
, even though N
has the type i32
at its declaration. This is surprising, I guess, but also makes sense from the implementation of the astconv code.
Alternative fix to this would be, uh, replacing this
with let impl1_trait_ref = tcx.impl_trait_ref(impl1_def_id).unwrap().subst(tcx, ty::InternalSubsts::identity_for_item(tcx, impl1_def_id)); Or making |
78f0613
to
f714613
Compare
r=me if CI passes |
@bors r=BoxyUwU |
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#106921 (Add documentation about the memory layout of `Cell`) - rust-lang#108828 (Emit alias-eq when equating numeric var and projection) - rust-lang#108834 (Do not ICE when we have fn pointer `Fn` obligations with bound vars in the self type) - rust-lang#108900 (fix(lexer): print whitespace warning for \x0c) - rust-lang#108930 (feat: implement better error for manual impl of `Fn*` traits) - rust-lang#108937 (improve readability of winnowing) - rust-lang#108947 (Don't even try to combine consts with incompatible types) - rust-lang#108976 (Update triagebot rust-analyzer team mention) - rust-lang#108983 (Forbid `#[target_feature]` on safe default implementations) Failed merges: - rust-lang#108950 (Directly construct Inherited in typeck.) r? `@ghost` `@rustbot` modify labels: rollup
I left a more detailed explanation for why this fixes this issue in the UI test, but in general, we should not try to unify const infer vars and rigid consts if they have incompatible types. That's because we don't want something like aConstArgHasType
predicate to suddenly go from passing to failing, or vice versa, due to a shallow resolve.type_of
for a parameter intry_eval_lit_or_param
, instead of the "expected" type from aWithOptConstParam
def id.Fixes #108781