use ProjectionPredicate instead of AliasRelate#158606
Conversation
| fn test<T: Foo1<Assoc1 = <T as Foo2>::Assoc2> + Foo2<Assoc2 = <T as Foo1>::Assoc1>>() { | ||
| needs_bar::<T::Assoc1>(); | ||
| //~^ ERROR: the trait bound `<T as Foo2>::Assoc2: Bar` is not satisfied | ||
| //~| ERROR: the size for values of type `<T as Foo2>::Assoc2` cannot be known at compilation time |
There was a problem hiding this comment.
I'm unsure about this one, to be honest. It makes sense why it can't prove Sized, it's divergent, but I don't know why printing a failure to prove sized was suppressed before, why recursive-self-normalization.rs does still not print this (which is direct recursion rather than a 2-cycle), or if the fact this is printing an error now is indicative of an underlying issue.
There was a problem hiding this comment.
This error might be related #155388? I personally wouldn't worry too much about emitting a new error here. Our error reporting logic is quite brittle with changes like this
This comment has been minimized.
This comment has been minimized.
| } | ||
| coercion.obligations.extend(ocx.into_pending_obligations()); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
fun, this was dead code before 😁
There was a problem hiding this comment.
haha yeah, saw this when I was deleting it and I was like "uuuh wait, why... AliasRelate in the old solver?? huh??" :P
| | ty::PredicateKind::NormalizesTo(_) => VisibleForLeakCheck::No, | ||
| ty::PredicateKind::AliasRelate(_, _, _) => VisibleForLeakCheck::Yes, | ||
| }; | ||
| let vis = VisibleForLeakCheck::No; |
There was a problem hiding this comment.
can inline that further 😁
This comment has been minimized.
This comment has been minimized.
d0b358b to
47e4f26
Compare
| /// However, in case the just-normalized `term` contains further aliases, we | ||
| /// emit nested `ProjectionPredicate` goals to normalize it further, which | ||
| /// are eagerly evaluated and may fail. |
There was a problem hiding this comment.
so i think the reason we get errors here is that term has not been just-normalized.
Because any alias we try to normalize when adding goals we should also have tried to normalize earlier. I would expect that by adding a normalize call to places where we currently don't normalize explicitly before calling this fn we'd stop panicking here.
However, I don't necessary want to expect that stuff is already normalized in this function, so not ICEing if the equate fails is still fine🤔
There was a problem hiding this comment.
right yeah, like, it's been "partially" normalized? e.g. <SomeStruct as std::ptr::Pointee>::Metadata --> <TailOfThatStruct as std::ptr::Pointee>::Metadata is a partial step in the progress of the entire expression being normalized (it's been "just-normalized" in that sense), but, TailOfThatStruct here has not been further normalized
There was a problem hiding this comment.
we normalized the original alias, but fetching an assoc type from an impl/or a struct field gives you a separate unnormalized type
There was a problem hiding this comment.
ok, pushed a rephrasing! I thiiink I've addressed all feedback now.
47e4f26 to
551a033
Compare
This comment has been minimized.
This comment has been minimized.
203f3b2 to
9b818d1
Compare
|
small feedback wrt to tests, I'd prefer to go //@ flags n stuff
// comments
sourcein UI tests because that makes it easier for me to notice flags where putting them in the middle always takes me a second to figure out after that r=me @bors delegate+ |
9b818d1 to
c77583e
Compare
|
@bors r=lcnr |
use ProjectionPredicate instead of AliasRelate part of rust-lang#155345 this PR is split into three commits: - refactoring `super_combine_{tys,consts}`, should not change behavior at all, just reorganizing code - convert to use ProjectionPredicate instead of AliasRelate in super_combine_tys and generalization (this is the important change for this PR) - deleting the now-dead AliasRelate predicate r? @lcnr
…uwer Rollup of 22 pull requests Successful merges: - #155429 (Support `u128`/`i128` c-variadic arguments) - #158100 (Emit retags in codegen to support BorrowSanitizer (part 4)) - #158494 (Improve E0277 diagnostics for conditionally implemented traits) - #158606 (use ProjectionPredicate instead of AliasRelate) - #158627 (Simplify option-iterator flattening in the compiler) - #158658 (Update LLVM submodule) - #158665 (Revert "Remove redundant dyn-compatibility check.") - #158021 (Remove old MinGW workaround) - #158473 (Add `riscv32imfc-unknown-none-elf` bare-metal target) - #158549 (process::exec: using appropriate exit code on vxworks.) - #158585 (Improve diagnostic for too many super keywords) - #158637 (hir_ty_lowering: avoid self type lookup for inherent aliases) - #158651 (ptr doc: reduce use of unsafe block to where needed) - #158669 (Remove `src/tools/test-float-parse/Cargo.lock`) - #158674 (library: Polish transmute's `split_at_stdlib` example) - #158677 (Add extra splat tests) - #158680 (Avoid ICE for `NonZero<char>` in improper_ctypes) - #158681 (Remove unnecessary `Hash` derives from MIR types) - #158682 (Avoid delayed bug for disabled on_type_error arguments) - #158684 (Add missing generic test coverage for ```#[splat]```) - #158687 (Streamline `MacEager`) - #158688 (Cleanup attribute docs and add links to other mentioned attributes)
View all comments
part of #155345
this PR is split into three commits:
super_combine_{tys,consts}, should not change behavior at all, just reorganizing coder? @lcnr