-
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
Uplift Relate
/TypeRelation
into rustc_next_trait_solver
#125724
Conversation
changes to the core type system changes to the core type system These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
edit: I just ended up uplifting relations into |
This comment has been minimized.
This comment has been minimized.
f9b3ff1
to
e293ba0
Compare
This comment has been minimized.
This comment has been minimized.
e293ba0
to
a084c35
Compare
compiler/rustc_type_ir/src/relate.rs
Outdated
b_arg: I::GenericArgs, | ||
) -> RelateResult<I, I::GenericArgs> | ||
where | ||
I::GenericArg: Relate<Infcx>, |
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.
Actually, I think this makes it pretty obvious that Relate
needs to be parameterized over Interner
, not InferCtxt
.
We want to be able to add the bound:
trait Interner {
type GenericArg: ... + Relate<Self>;
// ++++++++++++
}
which we can't do if Relate
is generic over Infcx
:/
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.
trait Sup {
type Assoc;
}
trait Trait: Sup<Assoc: Copy> {}
fn foo<T: Trait>(x: T::Assoc) -> (T::Assoc, T::Assoc) {
(x, x)
}
tbh I am actually surpised that that works, don't fully understand why it does
Can add
trait Interner {
type GenericArg;
}
// To allow an impl in `rustc_infer` while still having super traits
trait InternerForInfcx<Infcx: ?Sized>: Interner<GenericArg: Relate<Infcx>> {}
trait InferCtxtLike {
type Interner: InternerForInfcx<Self>;
}
fn foo<Infcx: InferCtxtLike<Interner = I>, I: InternerForInfcx<Infcx>>() {}
:3
edit: this will make rustc a ParamEnv
stress test however, because now every function with an InternerForInfcx<Infcx>
bound will have a lot of caller_bounds
@rustbot author |
☔ The latest upstream changes (presumably #125671) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
stopped reviewing after getting to the comment in relate.rs
as a general note: would you mind moving some of the param bounds into where-bounds, I personally find multi-line generic param listings hard to read
|
||
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] | ||
#[cfg_attr(feature = "nightly", derive(HashStable_NoContext, TyEncodable, TyDecodable))] | ||
pub enum BoundConstness { |
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.
idk how I feel about BoundConstness
being part of const_kind
, maybe move it to const_trait
module or sth?
/// type). | ||
CyclicTy(I::Ty), | ||
CyclicConst(I::Const), | ||
ProjectionMismatched(ExpectedFound<I::DefId>), |
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.
ProjectionMismatched(ExpectedFound<I::DefId>), | |
ProjectionMismatch(ExpectedFound<I::DefId>), |
idk if you want to fix it in this PR
#[derive(TypeVisitable_Generic)] | ||
#[rustc_pass_by_value] | ||
pub enum TypeError<I: Interner> { | ||
Mismatch, |
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.
not for this PR
Mismatch, | |
Misc, |
/// Returns a static string we can use for printouts. | ||
fn tag(&self) -> &'static str; |
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.
not necessarily for this PR. Remove this and just use std::any::type_name
for debug printing instead
compiler/rustc_type_ir/src/relate.rs
Outdated
b_arg: I::GenericArgs, | ||
) -> RelateResult<I, I::GenericArgs> | ||
where | ||
I::GenericArg: Relate<Infcx>, |
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.
trait Sup {
type Assoc;
}
trait Trait: Sup<Assoc: Copy> {}
fn foo<T: Trait>(x: T::Assoc) -> (T::Assoc, T::Assoc) {
(x, x)
}
tbh I am actually surpised that that works, don't fully understand why it does
Can add
trait Interner {
type GenericArg;
}
// To allow an impl in `rustc_infer` while still having super traits
trait InternerForInfcx<Infcx: ?Sized>: Interner<GenericArg: Relate<Infcx>> {}
trait InferCtxtLike {
type Interner: InternerForInfcx<Self>;
}
fn foo<Infcx: InferCtxtLike<Interner = I>, I: InternerForInfcx<Infcx>>() {}
:3
edit: this will make rustc a ParamEnv
stress test however, because now every function with an InternerForInfcx<Infcx>
bound will have a lot of caller_bounds
a084c35
to
05abaed
Compare
☔ The latest upstream changes (presumably #125775) made this pull request unmergeable. Please resolve the merge conflicts. |
05abaed
to
99999b3
Compare
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
I think this is basically ready for another round of review @rustbot ready |
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
☔ The latest upstream changes (presumably #125976) made this pull request unmergeable. Please resolve the merge conflicts. |
99999b3
to
e99efa6
Compare
going to wait on #125958 with this (if we're able to merge that today) |
Yeah I already locally rebased it onto that PR expecting it would land first, will push the rebased one when it's done. |
☔ The latest upstream changes (presumably #125958) made this pull request unmergeable. Please resolve the merge conflicts. |
e99efa6
to
91274c8
Compare
@bors r=lcnr |
Uplift `Relate`/`TypeRelation` into `rustc_next_trait_solver` For use in the new solver. This doesn't yet uplift `ObligationEmittingRelation`. r? lcnr
…kingjubilee Rollup of 8 pull requests Successful merges: - rust-lang#125606 (Size optimize int formatting) - rust-lang#125724 (Uplift `Relate`/`TypeRelation` into `rustc_next_trait_solver`) - rust-lang#126040 (Don't warn on fields in the `unreachable_pub` lint ) - rust-lang#126065 (mark binding undetermined if target name exist and not obtained) - rust-lang#126098 (Remove `same-lib-two-locations-no-panic` run-make test) - rust-lang#126099 (Crate loader cleanups) - rust-lang#126101 (Revert "Disallow ambiguous attributes on expressions" on nightly) - rust-lang#126103 (Improve Docs for `hir::Impl` and `hir::ImplItem`) r? `@ghost` `@rustbot` modify labels: rollup
…kingjubilee Rollup of 7 pull requests Successful merges: - rust-lang#125606 (Size optimize int formatting) - rust-lang#125724 (Uplift `Relate`/`TypeRelation` into `rustc_next_trait_solver`) - rust-lang#126040 (Don't warn on fields in the `unreachable_pub` lint ) - rust-lang#126098 (Remove `same-lib-two-locations-no-panic` run-make test) - rust-lang#126099 (Crate loader cleanups) - rust-lang#126101 (Revert "Disallow ambiguous attributes on expressions" on nightly) - rust-lang#126103 (Improve Docs for `hir::Impl` and `hir::ImplItem`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125724 - compiler-errors:uplift-relate, r=lcnr Uplift `Relate`/`TypeRelation` into `rustc_next_trait_solver` For use in the new solver. This doesn't yet uplift `ObligationEmittingRelation`. r? lcnr
For use in the new solver. This doesn't yet uplift
ObligationEmittingRelation
.r? lcnr