Seems to rely on behavior related to known soundness bug

[steffahn]

This crate (and also similarly polonius-the-crab) work with a

pub trait HKT 
where
    Self: for<'any> WithLifetime<'any>, 
{ }
pub trait WithLifetime<'lt> {
    type T;
}

trait, and dyn for<'a> WithLifetime<'a, T = Foo<'a>> trait objects that implement it.

However, such trait objects currently are able to implement the trait in ways that a manual implementation never could. E.g. try replicating dyn for<'a> WithLifetime<'a, T = &'a T> for some type T that does not fulfill T: 'static.

Code example and error message (click to expand)…

--- ```rs pub trait HKT where Self: for<'any> WithLifetime<'any>, { } pub trait WithLifetime<'lt> { type T; } use std::convert::Infallible; use std::marker::PhantomData; struct HktRef(PhantomData T>, Infallible); impl<'a, T> WithLifetime<'a> for HktRef { type T = &'a T; } ``` ``` error[E0309]: the parameter type `T` may not live long enough --> src/lib.rs:15:14 | 15 | type T = &'a T; | ^^^^^ ...so that the reference type `&'a T` does not outlive the data it points at | help: consider adding an explicit lifetime bound... | 14 | impl<'a, T: 'a> WithLifetime<'a> for HktRef { | ++++ For more information about this error, try `rustc --explain E0309`. ``` ---

And such impossible-to-implement-manually examples are used throughout the crate, e.g. in the implementation of .map_to_ref to name just the first example I could find.

The relevant soundness issue is demonstrated here.

I’m not saying that typical usage of polonius-the-crap or lending-iterator as intended is necessarily prone to being unsound (though I haven’t really thought too deeply yet about whether or not that’s the case). However, the heavy usage of such trait objects seems “risky” at least in the sense that an eventual bugfix of rust-lang/rust#84533 might break these crates.

I think these crates have enough potential for being useful, so that this risk of breakage might eventually apply to a large number of crates that depend on them, which seems like something we should try to avoid.

---

IMO, with the current status of soundness issues, the “safest” approach would be to try and avoid using trait objects alltogether, even though I admit the trait-object-types-as-HKTs hack is highly convenient (every manual-trait-impls approach would presumably involve the need for listing free variables as well as their relevant trait bounds in a HKT! macro).

[danielhenrymantilla]

polonius-the-crap

😭

---

Thanks for the report, I have to admit I was initially surprised the first time I noticed that dyn for<'a> WithLifetime<'a, T = &'a T> would work without the need for "upper bounds" 😅. But, in practice, this is indeed what managed to make these crates —especially lending-iterator!— actually usable, since without 'static bounds in many places, as you said, we'd have to be carrying upper bounds around.

FWIW, I've transposed your PoC of the unsoundness using WithLifetime & HKT:

```rs use ::lending_iterator::higher_kinded_types::*; trait ConversionProof<'a, 'b> : Sized { fn convert ( proof: [Self; 0], x: &'a T, ) -> &'b T ; } impl<'a, 'b> ConversionProof<'a, 'b> for &'b &'a () { fn convert ( _proof: [Self; 0], x: &'a T, ) -> &'b T { x } } fn convert<'a, 'b, T, O : HKT> ( x: &'a T, ) -> &'b T where Apply!(O<'a>) : ConversionProof<'a, 'b>, { O::T::convert([], x) } pub fn extend_lifetime<'a, 'b, T> ( x: &'a T, ) -> &'b T { convert::<_, HKT!(&'b &'_ ())>(x) } ``` - (I've skipped the `T : ?Sized` loosening to keep the snippet more lightweight, but it would also work with that unbound)

---

Regarding potential breakage in the future

• polonius-the-cra{b,p} used not to rely on this dyn trick, so we could make a revert, although it would require the extra "please provide the generics in scope" lack of ergonomics.

  In general, since that cra{te,p} hides the dyn for<'a> WithLifetime<'a under the rug of the polonius…! macros, I do suspect that slapping an upper-bound to limit the for<'a quantification would be doable (I think I had this at some draft stage; imagine the polonius function taking an extra upper-bound lifetime like thread::scoped() does).

  All that to say that I'm quite confident I could make a patch update of that crate to dodge the issue (or even better, we could go and try to anticipate it!).

• lending-iterator is definitely a pickle, in this regard, since we do have to expose direct HKT! usage to users, and that usage currently does not expect them to provide "upper bounds for the universal quantification", and luckily so, since at that point the API would become unintelligible but for the most lifetime-savy people, thence killing the whole crate altogether, I'd say (but for a standalone windows_mut, I guess).

  My hope, in this regard, thus, is for generic_associated_types and/or properly-quantified for<…> bounds and/or existential parameters (here, the upper-bound lifetime) to be available —before that soundness bug is patched in a way that would make dyn for<'a> Trait… : for<'a> Trait… no longer hold (or the type altogether to no longer exist!)—, so as to be able to fix this crate with a patch update. But I'd be keen to hear what @nikomatsakis or other lang people working on lifetimes would have to say about this case: maybe the necessary higher-order abstraction to be able to soundly express LendingIterator::map semantics concisely will never be made available to Rust?

  • Assuming HKT!(<'a> => &'a T) to still exist, but for it to implement, for<'a where 'T : 'a> WithLifetime<'a> = HKT<'T> (assuming 'T to be a thing, at least existentially), then we could envision LendingIterator::map's signature to become:

    fn map<'upper, Ret : HKT<'upper>, F>…
    where
         F : for<'n> FnMut([&'n (Self, &'upper ()); 0], Self::Item<'n>) -> A!(Ret<'n>),

    and thanks to lifetime inference I'd imagine this to be compatible with the most frequent currently-written call-sites that we have now (I guess I could try to conservatively introduce 'upper and tweak the array parameter to include it, just in case?).

    In this scenario, I do see a way forward with almost no breakage 🙏

[steffahn]

polonius-the-crap

:sob:

OMG, sorry, what a typo 🤣