-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Emit alias-eq when equating numeric var and projection #108828
Emit alias-eq when equating numeric var and projection #108828
Conversation
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.
r=me after nits
3aa876c
to
8b43968
Compare
This comment has been minimized.
This comment has been minimized.
8b43968
to
06ae044
Compare
06ae044
to
d4b59a0
Compare
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
@@ -148,10 +148,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
rhs_ty, | |||
op, | |||
); | |||
self.demand_suptype(expr.span, builtin_return_ty, return_ty); | |||
self.demand_eqtype(expr.span, builtin_return_ty, return_ty); |
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.
This demand_suptype
doesn't matter because the only types we're gonna encounter here are scalars.
@@ -148,10 +148,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
rhs_ty, | |||
op, | |||
); | |||
self.demand_suptype(expr.span, builtin_return_ty, return_ty); | |||
self.demand_eqtype(expr.span, builtin_return_ty, return_ty); | |||
builtin_return_ty |
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.
But returning builtin_return_ty
here means that when we equate _#0i
and _#1i
, the type we return will be _#0i
instead of <#_0i as Add<_#i1>>::Output
, which helps guide inference in the solver.
CanonicalVarInfo { kind: CanonicalVarKind::Ty(CanonicalTyVarKind::Float) }, | ||
t, | ||
), | ||
ty::Infer(ty::IntVar(vid)) => { |
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.
We also needed to fix the canonicalizer bug for int and float vars as well...
@@ -15,41 +15,65 @@ LL | second == 1 | |||
= note: expected fn item `fn() {second}` | |||
found type `{integer}` | |||
|
|||
error[E0308]: mismatched types |
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.
Due to the changes to typeck, we end up emitting this additional error. I can work around it pretty easily by checking if the output type of the binop references errors before returning builtin_return_ty
a few files up, but I also think that it's fine? Like, this is a meaningful error 😅
@bors r+ |
…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
This doesn't fix everything having to do with projections and infer vars, but it does fix a common case I saw in HIR typeck.
r? @lcnr