Repository navigation
Remove the At struct in favor of just splatting the fields into function parameters. - #163448
fallible-algebra wants to merge 10 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
|
changes to the core type system cc @lcnr Some changes occurred in engine.rs, potentially modifying the public API of cc @lcnr Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor
cc @rust-lang/clippy |
|
rustbot has assigned @JonathanBrouwer. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
|
A lot of the |
| if infcx | ||
| .at(&cause, param_env) | ||
| .relate(DefineOpaqueTypes::Yes, source_ty, ty::Variance::Invariant, target_ty) | ||
| .relate_at( |
There was a problem hiding this comment.
jup, please just infcx.relate. no need for the at 😁
| let impl_ty = self.normalize(span, tcx.type_of(impl_def_id).instantiate(tcx, args)); | ||
| let self_ty = self.normalize(span, Unnormalized::new_wip(self_ty)); | ||
| match self.at(&self.misc(span), self.param_env).eq( | ||
| match self.eq_at( |
There was a problem hiding this comment.
i'd like us to be consistent with the order of fields
I think normalize does span, relevant data and looking at ObligatioNCtxt, it's Span, env, data. So please change all methods to also have that order 😊
| expression_ty | ||
| }) | ||
| fcx.eq_at( | ||
| // needed for tests/ui/type-alias-impl-trait/issue-65679-inst-opaque-ty-from-val-twice.rs |
There was a problem hiding this comment.
comment should stay on DefineOpaqueTypes::Yes
also. Why are we using at here. We should be using some FnCtxt::eq method 🤔 we have FnCtxt::demand_eq but no eq which doesn't eagerly error. I guess that makes sense as eq outside of a probe always taints the root context. I guess that's separate from this PR 😁 so nothing to do here
| infcx: self.infcx, | ||
| cause: self.cause, | ||
| param_env: self.param_env, | ||
| infcx: &self, |
There was a problem hiding this comment.
why &self 🤔 shouldn't self already be &InferCtxt?
36ab6d4 to
48dcf59
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b538be3 to
99a7619
Compare
This comment has been minimized.
This comment has been minimized.
|
(feel free to reroll a reviewer when this is ready) |
99a7619 to
a7fd6b3
Compare
|
@lcnr the |
|
That test was actually written by me 😅 How does it fail? It's a test with compilation failure after all 🤔 |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors try jobs=test-aarch64-apple-1,test-aarch64-apple-2 |
This comment has been minimized.
This comment has been minimized.
Remove the `At` struct in favor of just splatting the fields into function parameters. try-job: test-aarch64-apple-1 try-job: test-aarch64-apple-2
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test for 4bbdb57 failed: CI. Failed job:
|
| expression_ty | ||
| }) | ||
| fcx.eq_at( | ||
| // needed for tests/ui/type-alias-impl-trait/issue-65679-inst-opaque-ty-from-val-twice.rs |
| // and don't allow converting between different structs, | ||
| // so there is no way this ever actually defines an opaque | ||
| // type. Thus choosing `Yes` is fine. | ||
| &cause, |
| &self.infcx, | ||
| Unnormalized::new_wip(hidden_type), | ||
| self.param_env, | ||
| &cause, |
| Unnormalized::new_wip(value), | ||
| universes, | ||
| self.fcx.param_env, | ||
| &cause, |
| /// Makes `actual <: expected`. For example, if type-checking a | ||
| /// call like `foo(x)`, where `foo: fn(i32)`, you might have | ||
| /// `sup(i32, x)`, since the "expected" type is the type that | ||
| /// appears in the signature. |
There was a problem hiding this comment.
don't remove the comment please :>
| value: Unnormalized<'tcx, T>, | ||
| fulfill_cx: &mut dyn TraitEngine<'tcx, E>, | ||
| param_env: ty::ParamEnv<'tcx>, | ||
| cause: &ObligationCause<'tcx>, |
| ty: Unnormalized<'tcx, Ty<'tcx>>, | ||
| fulfill_cx: &mut dyn TraitEngine<'tcx, E>, | ||
| param_env: ParamEnv<'tcx>, | ||
| cause: &ObligationCause<'tcx>, |
| ct: Unnormalized<'tcx, ty::Const<'tcx>>, | ||
| fulfill_cx: &mut dyn TraitEngine<'tcx, E>, | ||
| param_env: ParamEnv<'tcx>, | ||
| cause: &ObligationCause<'tcx>, |
| term: Unnormalized<'tcx, ty::Term<'tcx>>, | ||
| fulfill_cx: &mut dyn TraitEngine<'tcx, E>, | ||
| param_env: ParamEnv<'tcx>, | ||
| cause: &ObligationCause<'tcx>, |
| ) | ||
| .map(|resolved| infcx.deeply_resolve_ignoring_regions(resolved.value).skip_binder()) | ||
| .unwrap_or(ty.skip_binder()); | ||
| if let Some(new_def_id) = ty.ty_adt_def().map(|adt| adt.did()) { |
There was a problem hiding this comment.
you pointed to that diff on zulip, why does it exist '^^
There was a problem hiding this comment.
Misclick on a rebase I imagine 😅
|
Closing this in favour of staggering the removal of |
View all comments
AKA "splat the
At"Draft as I still need to rebase.