Remove 'static requirement on try_as_dyn

[oli-obk]

View all comments

tracking issue: #144361

cc @ivarflakstad @izagawd

[rustbot]

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[rustbot]

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[danielhenrymantilla]

Some drive-by comments; but I'm not rustc/HIR-savy, so take these with a grain of salt 🙇

View changes since this review

[BoxyUwU]

r? BoxyUwU

[oli-obk]

The hacky solution is obviously not a general fix. But I think it's progress. As a next step I will add the input type as a generic parameter on TryAsDynCompat, at which point we should be able to enforce (in borrowck) that the input type outlives any lifetimes on the dyn Trait or its generic parameters. So if a generic parameter T has a 'static bound, it could be used as an input type for a try_as_dyn irrespective of the bounds on the dyn Trait. In the other direction, we will likely end up rejecting many traits that have generic parameters as the bounds are not something that can be written in Rust.

[oli-obk]

Thanks @danielhenrymantilla for writing down this summary. I haven't addressed the concern about the restriction yet, but I included your summary mostly verbatim in the unstable book

[theemathas]

This would already run into the "no 'static rule". I'm unsure if this rule is needed or not.

[danielhenrymantilla]

Very true, let's go with the following example, then:

impl<'a, 'b> Trait<'b> for &'a str
where
    Option<&'a str>: IntoIterator<Item = &'b str>,
{}

which amounts to having done:

impl<'a> Trait<'a> for &'a str {}

which brings us back to "no repetitions in header" (&'? str: Trait<'static> discriminates '? against 'static).

---

Suggested change

	Checking whether `Option<&'? str>: IntoIterator<Item = &'static str>` holds discriminates `'?` against `'static`.
	Associated-type equality bounds can very much amount to lifetime-infected parameter equality constraints, which are problematic as per the "at most one mention of each lifetime-infected parameter in header" rule.
	To illustrate, with the following definitions, `&'? str: Trait<'static>` discriminates `'?` against `'static`:
	```rust
	impl<'a, 'b> Trait<'b> for &'a str
	where
	// &'a str = &'b str,
	Option<&'a str>: IntoIterator<Item = &'b str>,
	{}
	impl<'a> Trait<'a> for &'a str {}
	```

[theemathas]

This page doesn't define "lifetime-infected parameter" anywhere.

[theemathas]

This PR's TypeId::trait_info_of method is unsound when combined with ptr_metadata and arbitrary_self_types_pointers. This unsoundness occurs because TypeId::trait_info_of does not check whether the trait is TryAsDynCompatible.

I'm unsure whether arbitrary_self_types_pointers is necessary for the unsoundness. See rust-lang/unsafe-code-guidelines#516.

The following code performs a use-after-free with this PR.

#![feature(type_info, ptr_metadata, arbitrary_self_types_pointers)]

use std::{
    any::TypeId,
    ptr::{self, DynMetadata},
};

type Payload = Box<i32>;

trait Trait {
    type Assoc;
    fn method(self: *const Self, value: Self::Assoc) -> &'static Payload;
}
struct Thing;
impl Trait for Thing {
    type Assoc = &'static Payload;
    fn method(self: *const Self, value: Self::Assoc) -> &'static Payload {
        value
    }
}

fn extend<'a>(payload: &'a Payload) -> &'static Payload {
    let metadata: DynMetadata<dyn Trait<Assoc = &'a Payload>> = const {
        TypeId::of::<Thing>()
            .trait_info_of::<dyn Trait<Assoc = &'a Payload>>()
            .unwrap()
            .get_vtable()
    };
    let ptr: *const dyn Trait<Assoc = &'a Payload> =
        ptr::from_raw_parts(std::ptr::null::<()>(), metadata);
    ptr.method(payload)
}

fn main() {
    let payload: Box<Payload> = Box::new(Box::new(1i32));
    let wrong: &'static Payload = extend(&*payload);
    drop(payload);
    println!("{wrong}");
}

Potential solutions include:

• Add a (): TryAsDynCompatible<T> bound to TypeId::TraitInfoOf
  • Alternatively, change the TryAsDynCompatible trait to be a bound like dyn Trait: TryAsDynCompatible<'a>, and have a separate SourceType: 'a bound. And add T: TryAsDynCompatible<'a> to TypeId::TraitInfoOf.
• Change TraitImpl::get_vtable to be unsafe.

[oli-obk]

change the TryAsDynCompatible trait to be a bound like dyn Trait: TryAsDynCompatible<'a>, and have a separate SourceType: 'a bound.

hmm... and desugar dyn Trait: TryAsDynCompatible<'a> to adding a constraint on the lifetime 'foo of the trait object? Bidirectional outlives bounds? It feels a bit icky to connect it that way, but I guess so does (): TryAsDynCompatible

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[oli-obk]

@rustbot ready

[rustbot]

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.