Eagerly normalize in generalize#151746
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
… r=<try> Eagerly normalize in generalize
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 92bb985 failed: CI. Failed job:
|
fun, we've got some jank in serde which we don't have in our ui test suite |
5bd20bb to
d0cdaae
Compare
This comment has been minimized.
This comment has been minimized.
d0cdaae to
fb5d5cf
Compare
This comment has been minimized.
This comment has been minimized.
fb5d5cf to
d96cab0
Compare
This comment has been minimized.
This comment has been minimized.
d96cab0 to
5611a89
Compare
|
changes to the core type system cc @lcnr |
|
@lcnr I think this fixes all issues we had. Only one test really regressed, some got a bunch better. Fixed the span on the one test we discussed. |
5611a89 to
3f90d08
Compare
This comment has been minimized.
This comment has been minimized.
|
@GuillaumeGomez is this another flaky test? |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d3da459): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.6%, secondary 0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.937s -> 474.9s (-0.01%) |
3f90d08 to
fd20cab
Compare
This comment has been minimized.
This comment has been minimized.
fd20cab to
3bcdac1
Compare
This comment has been minimized.
This comment has been minimized.
3bcdac1 to
a335a41
Compare
This comment has been minimized.
This comment has been minimized.
a335a41 to
29ae136
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
29ae136 to
1adea68
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| /// `<T as Bar<<?0 as Foo>::Assoc>::Assoc == ?0`. This equality can | ||
| /// hold by either normalizing the outer or the inner associated type. | ||
| in_alias: bool, | ||
| state: GeneralizerState, |
There was a problem hiding this comment.
outdated comment, refer to the docs of GeneralizerState
| #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] | ||
| enum GeneralizerState { | ||
| /// Treat aliases as potentially normalizable. | ||
| Default, | ||
| IncompletelyRelateHigherRankedAlias, | ||
| /// Only one layer | ||
| ShallowStructurallyRelateAliases, | ||
| StructurallyRelateAliases, | ||
| } |
There was a problem hiding this comment.
please add more context here
- when are we in which state
- how do they impact behavior
or maybe just refer to handle_alias_ty and rely on the actual control-flow in that function
|
|
||
| match self.state { | ||
| GeneralizerState::IncompletelyRelateHigherRankedAlias => { | ||
| if self.infcx.next_trait_solver() |
There was a problem hiding this comment.
i feel like it'd be easier for us to only ever be in IncompletelyRelateHigherRankedAlias if we're in the new solver, so that we don't have to check this. In the old solver Default and IncompletelyRelateHigherRankedAlias are equivalent, are they not?
| span: Span, | ||
| alias: ty::AliasTy<'tcx>, | ||
| ) -> Ty<'tcx> { | ||
| let infcx = unsafe { |
There was a problem hiding this comment.
do you want to encapsulate these transmute in inhernet functions on InferCtxt and add a safety comment there?
| @@ -12,7 +13,6 @@ impl Trait for u32 {} | |||
| fn hello() -> Box<impl Trait> { | |||
| if true { | |||
| let x = hello(); | |||
| //[next]~^ ERROR: the size for values of type `dyn Trait` cannot be known at compilation time | |||
| let y: Box<dyn Trait> = x; | |||
| } | |||
There was a problem hiding this comment.
hmm, I don't fully get that 🤔
What does "implicitly contains the + Sized bound" mean? For coerce to consider that Sized bound, it has to be in the root fulfillment context I think?
How does that Sized bound get there. I am also surprised that x has to be equal to the return type of hello() instead of a subtype, how does that happen? ✨
View all comments
r? @lcnr
cc: rust-lang/trait-system-refactor-initiative#262