improve normalization of `Pointee::Metadata`
This PR makes it so that <Wrapper<Tail> as Pointee>::Metadata is normalized to <Tail as Pointee>::Metadata if we don't know Wrapper<Tail>: Sized. With that, the trait solver can prove projection predicates like <Wrapper<Tail> as Pointee>::Metadata == <Tail as Pointee>::Metadata, which makes it possible to use the metadata APIs to cast between the tail and the wrapper:
#![feature(ptr_metadata)]
use std::ptr::{self, Pointee};
fn cast_same_meta<T: ?Sized, U: ?Sized>(ptr: *const T) -> *const U
where
T: Pointee<Metadata = <U as Pointee>::Metadata>,
{
let (thin, meta) = ptr.to_raw_parts();
ptr::from_raw_parts(thin, meta)
}
struct Wrapper<T: ?Sized>(T);
fn cast_to_wrapper<T: ?Sized>(ptr: *const T) -> *const Wrapper<T> {
cast_same_meta(ptr)
}
Previously, this failed to compile:
error[E0271]: type mismatch resolving `<Wrapper<T> as Pointee>::Metadata == <T as Pointee>::Metadata`
--> src/lib.rs:16:5
|
15 | fn cast_to_wrapper<T: ?Sized>(ptr: *const T) -> *const Wrapper<T> {
| - found this type parameter
16 | cast_same_meta(ptr)
| ^^^^^^^^^^^^^^ expected `Wrapper<T>`, found type parameter `T`
|
= note: expected associated type `<Wrapper<T> as Pointee>::Metadata`
found associated type `<T as Pointee>::Metadata`
= note: an associated type was expected, but a different one was found
(Yes, you can already do this with as casts. But using functions is so much :sparkles: safer :sparkles:, because you can't change the metadata on accident.)
r? @b-naber
(rustbot has picked a reviewer for you, use r? to override)
Some changes occurred to the CTFE / Miri engine
cc @rust-lang/miri
r? types
I replaced predicate_must_hold_considering_regions with predicate_must_hold_modulo_regions and added some docs to warn about predicate_must_hold_considering_regions.
r? @lcnr @rustbot ready
The Miri subtree was changed
cc @rust-lang/miri
Some changes occurred to the core trait solver
cc @rust-lang/initiative-trait-system-refactor
:umbrella: The latest upstream changes (presumably #120660) made this pull request unmergeable. Please resolve the merge conflicts.
I was hoping that this PR would be sufficiently motivated by just the
cast_same_metaexample from the OP and "let's not break code that currently compiles", but evidently that's not the case, so here we are.
I agree that cast_same_meta should work, not sold on "let's not break code that currently compiles". I would expect that only using the adt tail for pointer casts won't break any actual code, even though it is theoretically breaking. I would very much prefer to try and break this for a "cleaner language" and only think about how to handle these overlapping impls if too many users are effected. We have crater for this.
I'd therefore be in favor of "Otherwise, I guess just normalizing to the metadata of the tail is an improvement over the status quo and all cases that it can break are easily fixed by adding where Tail: Sized."
@rustbot author
I've updated the OP to show the differences between before and after this PR. I'm still not convinced that the "fallback impl" is better than a specialization, but I guess it doesn't matter too much. :shrug:
@rustbot ready
I've updated the OP to show the differences between before and after this PR. I'm still not convinced that the "fallback impl" is better than a specialization, but I guess it doesn't matter too much. :shrug:
Neither am I. We already had that impl where we check for Sized for aliases and params. It is pretty much just an AliasBound candidate instead of an actual impl.
Pretty much what happens here is that if there's a Sized bound in the environment, either directly via T: Sized or indirectly from an item bound of T: Trait with type Assoc: Sized, we assume this means Pointee<Metadata = ()>. This is something the caller should prove. The only way you have an alias or an opaque as the self type (at least in the new solver) is if they are rigid as normalizing them relies on the environment.
We hackily reimplemented a super trait bound: trait Sized: Pointee<Metadata = ()> {}. I would like us to remove this manual builtin impl and instead add that super trait bound. That doesn't have to happen inside of this PR. I feel like we may have even considered to do so in the past but wasn't able to find anything skimming through previous PRs.
@lukas-code would you like to change it to a super trait right away or should we leave it to a future PR? If you want to not do so rn, please add a FIXME for it to the "fallback impl" arm in both solvers
Making Pointee<Metadata = ()> a supertrait of Sized seems like a really good idea. I'll implement that and see what breaks 😆 .
To answer the question of what breaks, it's this test: (in both solvers) https://github.com/rust-lang/rust/blob/4a2fe4491ea616983a0cf0cbbd145a39768f4e7a/tests/ui/sized/recursive-type-pass.rs#L1-L10
But that's probably fine, because that test compiling at all seems more like a side-effect of Sized being coinductive rather than intentional behavior. All other tests that rely on Sized being coinductive still pass.
But there are also a ton of diagnostics regressions, mostly complaining about "type mismatch resolving <Foo as Pointee>::Metadata == ()" in addition to "Foo is not sized". Fixing those seems to be non-trivial, so I'd like to move making Pointee<Metadata = ()> a supertrait of Sized to a future PR.
@rustbot ready
thanks for your good work and for staying with me while I tried to figure this out myself :heart:
@bors r+
:pushpin: Commit 15ffe839bacebb2883fd77959df319eebefaf8cf has been approved by lcnr
It is now in the queue for this repository.