Remove 'static requirement on try_as_dyn

[oli-obk]

tracking issue: #144361

cc @theemathas can you find an unsoundness when combined with generic impls where there are no lifetimes, but reference other impls that have lifetimes with constraints? I couldn't find anything.

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

[danielhenrymantilla]

This one is a "regression", right? With the old design we could be involving a &'static u32 which is both : 'static, and implementing Trait in the classical sense. Perhaps we'll want a try_as_dyn_static() function as well, for those interested in this use case?

[oli-obk]

I'll add this to the tracking issue

[danielhenrymantilla]

This is way over my head, lol, but just to sanity-check: could this branch be hoisted to the beginning of the function, as in, any impl that is not fully_generic_for_reflection(), when checking in Reflection mode, ought to result in Err(NoSolution), right? As in, it's not specific to this double-Negative branch?

[oli-obk]

Well, it's only relevant if the polarities match. Otherwise it's not a match anyway. So while it could be lifted to the front, it's not really necessary either. No strong opinion, but also easily changeable without affecting behavior

[BoxyUwU]

r? BoxyUwU

[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.

[oli-obk]

I don't have a solution for this yet. The easy hammer is to reject traits that have such methods. Nicer would be to figure out how to make a bound that ensures the dyn Trait's lifetimes are shorter than the arguments'

[BoxyUwU]

Can we check during mir_typeck that the source type outlives the lifetime of the trait object type?

[BoxyUwU]

ah its not the intrinsic 🤔 yeah thats tricky

[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.

[theemathas]

The current PR state is still unsound, because it doesn't check where Self: 'static bounds in supertraits. The following code segfaults with this PR.

#![feature(try_as_dyn)]

use std::any::try_as_dyn;

type Payload = Box<i32>;

trait Super {
    fn as_static(&self) -> &'static Payload
    where
        Self: 'static;
}

trait Sub: Super {}

impl<'a> Super for &'a Payload {
    fn as_static(&self) -> &'static Payload
    where
        Self: 'static,
    {
        *self
    }
}

impl<'a> Sub for &'a Payload {}

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

fn extend(a: &Payload) -> &'static Payload {
    let b: &(dyn Sub + 'static) = try_as_dyn::<&Payload, dyn Sub + 'static>(&a).unwrap();
    b.as_static()
}

[theemathas]

The PR is also unsound in another way: It does not check for the existence of an implied Self: 'static bound. The following code segfaults with this PR, using the dyn-clone crate version 1.0.20.

#![feature(try_as_dyn)]

use std::any::try_as_dyn;

use dyn_clone::{DynClone, clone_box};

type Payload = Box<i32>;

trait Trait: DynClone {
    fn as_static(&'static self) -> &'static Payload;  // &'static self has an implied Self: 'static bound
}

impl<'a> Trait for &'a Payload {
    fn as_static(&'static self) -> &'static Payload {
        *self
    }
}

fn extend<'a>(x: &'a Payload) -> &'static Payload {
    let dyn_ref: &(dyn Trait + 'static) =
        try_as_dyn::<&'a Payload, dyn Trait + 'static>(&x).unwrap();
    let dyn_box: Box<dyn Trait + 'static> = clone_box(dyn_ref);
    let dyn_static: &'static (dyn Trait + 'static) = Box::leak(dyn_box);
    dyn_static.as_static()
}

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

[theemathas]

Another unsoundness: Seems like some lifetimes were not checked? The following code segfaults with this PR.

#![feature(try_as_dyn)]

use std::any::try_as_dyn;

type Payload = Box<i32>;

trait Trait<'a> {
    fn as_payload(&self) -> &'a Payload;
}

impl<'a> Trait<'a> for &'a Payload {
    fn as_payload(&self) -> &'a Payload {
        *self
    }
}

fn extend<'a>(x: &'a Payload) -> &'static Payload {
    let dyn_ref: &(dyn Trait<'static> + 'static) =
        try_as_dyn::<&'a Payload, dyn Trait<'static> + 'static>(&x).unwrap();
    dyn_ref.as_payload()
}

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

[oli-obk]

Yes I know 😅 its why I wrote it's not a fix. All the examples you are giving would be fixed by actually checking lifetimes at the call site, which I'm planning as per my next comment

[theemathas]

we should be able to enforce (in borrowck) that the input type outlives any lifetimes on the dyn Trait or its generic parameters

Why is an "outlives" requirement sufficient, as opposed to a "lifetimes are all equal to each other" requirement?