TransmuteFrom: Gracefully handle unnormalized types and normalization errors #131112
Conversation
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
So I think this PR is overengineered 😅 I left some inline comments, but realized that they're kinda ignoring the fact that this isn't the right general fix, I think.
Factoring out the confirmation code may be useful, but right now it's tied together with unnecessary/incorrect normalization calls which mask the real issue here:
The correct fix is just to add checks in get_safe_transmute_error_and_reason for the invariants that the transmutability code currently relies on. Specifically, we should be checking the same things as we do in assemble_candidates_for_transmutability -- i.e.
if obligation.predicate.has_non_region_param()
since right now, the transmutability code doesn't handle type/const parameters, and
if obligation.has_non_region_infer()
which implies ambiguity.
| use rustc_infer::infer::TyCtxtInferExt; | ||
|
|
||
| use crate::traits::ObligationCtxt; | ||
| let infcx = tcx.infer_ctxt().with_next_trait_solver(true).build(); |
There was a problem hiding this comment.
Making a new inference context is definitely not the right approach.
|
Also unrelated, I believe the transmutability code should be resilient to #![feature(transmutability)]
trait A {
type AssocA;
}
trait B {
type AssocB: std::mem::TransmuteFrom<()>;
}
impl<T> B for T
where
for<'a> &'a i32: A,
{
type AssocB = <&'static i32 as A>::AssocA;
} |
This comment has been minimized.
This comment has been minimized.
| @@ -2221,6 +2224,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { | |||
| ) -> GetSafeTransmuteErrorAndReason { | |||
| use rustc_transmute::Answer; | |||
|
|
|||
| if obligation.predicate.has_non_region_param() || obligation.has_non_region_infer() { | |||
| where | ||
| T: A, | ||
| { | ||
| type AssocB = T::AssocA; //~ERROR: the trait bound `<T as A>::AssocA: TransmuteFrom<(), Assume { alignment: false, lifetimes: false, safety: false, validity: false }>` is not satisfied [E0277] |
There was a problem hiding this comment.
I'm personally fine to punt on making this error message prettier — my top priority is making TransmuteFrom work — but happy to try to improve this further if you'd like to block the PR on it.
There was a problem hiding this comment.
No, it's fine. But yeah, it could be customized to say something like "cannot transmute types with ty/ct params in it".
There was a problem hiding this comment.
But happy to do that later.
|
@compiler-errors, you're right, that was a lot more simple: https://github.com/rust-lang/rust/pull/131112/files |
jswrenn
changed the title
TransmuteFrom: normalize types, unify confirmation and error reporting
|
Also please run |
This comment has been minimized.
This comment has been minimized.
|
@rustbot author |
rustbot
added
S-waiting-on-author
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#131024 (Don't give method suggestions when method probe fails due to bad implementation of `Deref`) - rust-lang#131112 (TransmuteFrom: Gracefully handle unnormalized types and normalization errors) - rust-lang#131176 (.gitignore files for nix) - rust-lang#131183 (Refactoring to `OpaqueTyOrigin`) - rust-lang#131187 (Avoid ICE in coverage builds with bad `#[coverage(..)]` attributes) - rust-lang#131192 (Handle `rustc_query_impl` cases of `rustc::potential_query_instability` lint) - rust-lang#131197 (Avoid emptiness check in `PeekMut::pop`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131112 - jswrenn:fix-130413, r=compiler-errors TransmuteFrom: Gracefully handle unnormalized types and normalization errors ~~Refactor to share code between `TransmuteFrom`'s trait selection and error reporting code paths. Additionally normalizes the source and destination types, and gracefully handles normalization errors.~~ Fixes rust-lang#130413 r? `@compiler-errors`
Refactor to share code betweenTransmuteFrom's trait selection and error reporting code paths. Additionally normalizes the source and destination types, and gracefully handles normalization errors.Fixes #130413
r? @compiler-errors