lifetimes with R* types break compared to non R* types #75

Licenser · 2022-01-31T16:05:54Z

Hi,
sorry for the bad title, I'm not sure how to phrase the issue in a concise one-line description.

It seems that something inside abi_stables R* types breaks rusts lifetime tracking. We (@marioortizmanero and the rest of the tremor team) went through the attempt of using them for internal data as part of his PDK project and got to the point where a lot of lifetime errors (the nasty kind) were thrown up.

Initially, we suspected that we had done something wrong in the interpreter using the data so we tried to find the underlying cause and boiled it down to (hopefully) one initial issue that doesn't require to go through 1000s of lines of code :)

Basically, the following code

use abi_stable::std_types::RCow;
use std::borrow::Cow;

fn cmp_cow<'a, 'b>(left: &Cow<'a, ()>, right: &Cow<'b, ()>) -> bool {
    left == right
}

fn cmp_rcow<'a, 'b>(left: &RCow<'a, ()>, right: &RCow<'b, ()>) -> bool {
    left == right
}

fn main() {
    println!("Hello, world!");
}

fails with the following error:

    Checking repro v0.1.0 (/home/heinz/mario/repro)
error[E0623]: lifetime mismatch
 --> src/main.rs:9:10
  |
8 | fn cmp_rcow<'a, 'b>(left: &RCow<'a, ()>, right: &RCow<'b, ()>) -> bool {
  |                            ------------          ------------
  |                            |
  |                            these two types are declared with different lifetimes...
9 |     left == right
  |          ^^ ...but data from `left` flows into `right` here

For more information about this error, try `rustc --explain E0623`.
error: could not compile `repro` due to previous error

We could reproduce the same for RVec, and RHashMap as long they contained a lifetime.

Below a more extensive test with a number of other types:

use abi_stable::std_types::{RCow, RHashMap, RSlice, RStr, RString, RVec, Tuple2};
use std::{borrow::Cow, collections::HashMap};

fn cmp_string<'a, 'b>(left: &String, right: &String) -> bool {
    left == right
}
fn cmp_rstring<'a, 'b>(left: &RString, right: &RString) -> bool {
    left == right
}

fn cmp_str<'a, 'b>(left: &'a str, right: &'b str) -> bool {
    left == right
}
fn cmp_rstr<'a, 'b>(left: &RStr<'a>, right: &RStr<'b>) -> bool {
    left == right
}

fn cmp_vec<'a, 'b>(left: &Vec<&'a str>, right: &Vec<&'b str>) -> bool {
    left == right
}
fn cmp_rvec<'a, 'b>(left: &RVec<&'a str>, right: &RVec<&'b str>) -> bool {
    left == right
}

fn cmp_tpl<'a, 'b>(left: &(&'a str, u8), right: &(&'b str, u8)) -> bool {
    left == right
}
fn cmp_rtpl<'a, 'b>(left: &Tuple2<&'a str, u8>, right: &Tuple2<&'b str, u8>) -> bool {
    left == right
}

fn cmp_slice<'a, 'b>(left: &[&'a str], right: &[&'b str]) -> bool {
    left == right
}
fn cmp_rslice<'a, 'b>(left: &RSlice<&'a str>, right: &RSlice<&'b str>) -> bool {
    left == right
}
fn cmp_cow<'a, 'b>(left: &Cow<'a, ()>, right: &Cow<'b, ()>) -> bool {
    left == right
}
fn cmp_rcow<'a, 'b>(left: &RCow<'a, ()>, right: &RCow<'b, ()>) -> bool {
    left == right
}

fn cmp_hashmap<'a, 'b>(left: &HashMap<&'a str, ()>, right: &HashMap<&'a str, ()>) -> bool {
    left == right
}
fn cmp_rhashmap<'a, 'b>(left: &RHashMap<u8, &'a str>, right: &RHashMap<u8, &'b str>) -> bool {
    left == right
}

fn main() {
    println!("Hello, world!");
}

The text was updated successfully, but these errors were encountered:

marioortizmanero · 2022-02-01T09:37:13Z

It's worth noting that this only happens with abi_stable. For example, beef also exports a Cow type, but that works fine. Same for halfbrown's HashMap:

use abi_stable::std_types::{RCow, RHashMap};
use std::{borrow::Cow, collections::HashMap};
use beef::Cow as BCow;
use halfbrown::HashMap as HHashMap;

fn cmp_cow<'a, 'b>(left: &Cow<'a, str>, right: &Cow<'b, str>) -> bool {
    left == right
}
fn cmp_bcow<'a, 'b>(left: &BCow<'a, str>, right: &BCow<'b, str>) -> bool {
    left == right
}
fn cmp_rcow<'a, 'b>(left: &RCow<'a, str>, right: &RCow<'b, str>) -> bool {
    left == right
}

fn cmp_hashmap<'a, 'b>(left: &HashMap<u8, &'a str>, right: &HashMap<u8, &'b str>) -> bool {
    left == right
}
fn cmp_hhashmap<'a, 'b>(left: &HHashMap<u8, &'a str>, right: &HHashMap<u8, &'b str>) -> bool {
    left == right
}
fn cmp_rhashmap<'a, 'b>(left: &RHashMap<u8, &'a str>, right: &RHashMap<u8, &'b str>) -> bool {
    left == right
}

Currently investigating if it's just the PartialEq implementation maybe? They differ:

Or for RHashMap:

marioortizmanero · 2022-02-01T09:56:27Z

Can confirm that the problem lies in the implementations of PartialEq for R* types. Will submit a PR soon.

marioortizmanero · 2022-02-01T18:24:05Z

Ok, so note that while I've fixed the snippet of code in this issue (which showcases PartialEq), it still fails for PartialOrd:

use abi_stable::std_types::{RCow, RHashMap, RSlice, RStr, RString, RVec, Tuple2};
use std::{borrow::Cow, collections::HashMap, cmp::Ordering};

fn cmp_cow<'a, 'b>(left: &Cow<'a, ()>, right: &Cow<'b, ()>) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rcow<'a, 'b>(left: &RCow<'a, ()>, right: &RCow<'b, ()>) -> Option<Ordering> {
    left.partial_cmp(right)
}

Which is strange, because the signatures are actually the same:

Interestingly enough, beef has a different one, which is what I expected it to be:

https://docs.rs/beef/latest/beef/generic/struct.Cow.html#impl-PartialOrd%3CCow%3C%27_%2C%20B%2C%20V%3E%3E

So it might be a case of compiler magic? Same for Ord:

fn cmp_cow<'a, 'b>(left: &Cow<'a, str>, right: &Cow<'b, str>) -> Ordering {
    left.cmp(right)
}
fn cmp_rcow<'a, 'b>(left: &RCow<'a, str>, right: &RCow<'b, str>) -> Ordering {
    left.cmp(right)
}
fn cmp_bcow<'a, 'b>(left: &beef::Cow<'a, str>, right: &beef::Cow<'b, str>) -> Ordering {
    left.cmp(right)
}

Maybe it has to do with BorrowOwned?

Edit: oh, yup. It's BorrowOwned's fault. Or rather, its associated lifetime. Why do we need it again, @rodrimati1992? Could it be made into plain impls?

rodrimati1992 · 2022-02-01T19:14:05Z

@marioortizmanero

Edit: oh, yup. It's BorrowOwned's fault. Or rather, its associated lifetime. Why do we need it again, @rodrimati1992? Could it be made into plain impls?

The lifetime's required to make the associated types in the three BorrowOwned impls work Don't know what you mean about "plain" impls.

marioortizmanero · 2022-02-01T19:29:06Z

Nevermind, sorry, I was trying to figure out how BorrowOwned worked. Still trying different ways to get the same behaviour in std and here. The main problem is BorrowOwned for sure, though.

So far I've narrowed it down to this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b752600ddd4b1e03e2fea2fa5fc1228b

marioortizmanero · 2022-02-02T00:08:22Z

Oh my god, okay, I'm not crazy. I've been banging my head against the wall all day trying to fix this. I've tried literally everything. And I have some confirmation that it is indeed unsolvable in the case of Ord:

fn test<'a, 'b>(left: &RCow<'a, u8>, right: &RCow<'b, u8>) -> Ordering {
    left.cmp(right)
}

At some point I decided to give up and ask for help in Rust's unofficial Discord server because they are absolute geniuses. And indeed it helped a lot: I was right that BorrowOwned was the issue, but I didn't know the concept of "subtyping and variance".

BorrowOwned makes it impossible to have comparisons with different lifetimes, because it makes the trait invariant:

impl<'a, B: ?Sized> Ord for RCow<'a, B>
where
    B: Ord + BorrowOwned<'a>,
{
    #[inline]
    fn cmp(&self, other: &RCow<'a, B>) -> Ordering {
        Ord::cmp(&**self, &**other)
    }
}

std's implementation doesn't bind Cow's lifetime to any trait in its implementation of Ord, so it's covariant:

impl<B: ?Sized> Ord for Cow<'_, B>
where
    B: Ord + ToOwned,
{
    #[inline]
    fn cmp(&self, other: &Self) -> Ordering {
        Ord::cmp(&**self, &**other)
    }
}

Not sure if you're familiar with those terms; I certainly wasn't. There's more info here. I still don't know much about it, so I can't provide any solutions. For now, I can only think of:

avoiding the comparison in the first place; or
removing BorrowOwned and having people use RCow<RStr> (which isn't that terrible); or
waiting for GATs so that the lifetime isn't binded to the trait, but to the RBorrowed type inside of it. I was told that GATs don't solve the issue completely, though.

Just wanted to give a quick update about today's suffering. I'll give another update some other day.

marioortizmanero · 2022-02-09T13:02:29Z

Even after understanding in detail why this happens, there aren't any solutions other than the ones I mentioned. In fact, there are less possible approaches because I'm now certain that GATs won't work.

I followed quite a few tutorials and guides about subtyping and variance and was able to more or less understand the topic. The problem was that none of these had an example equivalent to abi_stable's, so I still didn't know why our specific example didn't work.

I first gave it a try with GATs: what if we binded the lifetime to the associated type instead of to the trait itself? To me the problem first seemed to be that we were introducing the lifetime 'a to specify that T: BorrowOwned<'a>. So if we managed to make BorrowOwned not take a lifetime, that could possibly fix it.

I'm not an expert in GATs because I haven't used them much, but I was finally able to get the following (simplified) interface working:

impl<T> BorrowOwned for T {
    type RBorrowed<'a> where T: 'a = &'a T;
}

See this playground link for the first approach I thought of, which is a direct translation of the original interface.
See this playground link for the final approach, which worked.

However, as I was warned, the interface itself compiled, but the lifetime issues were still there. I had now seen with my own eyes that the problem was invariance. Afterwards, I finally found a reference as to why this happens for our specific case:

https://rustc-dev-guide.rust-lang.org/variance.html#variance-and-associated-types

The problem is that it's documented on the Rust developer book, and not in the Rustnomicon or the reference. Only compiler developers are meant to check that book. And it makes sense, because I don't really understand the explanation; it's stuff related to how the compiler works.

Anyway, it simply says that "traits with associated types must be invariant with respect to all of their inputs". BorrowOwned needs associated types to be useful, but that makes its inputs invariant. Which mess up the lifetimes in many places, such as Ord. But it could also happen in many other situations, so it's a big problem that should be fixed.

There is one workaround, and that's using mem::transmute (thanks to Heinz for the idea):

fn compare<'a, 'b>(left: &RCow<'a, str>, right: &RCow<'b, str>) -> Ordering {
    unsafe {
        let right: &RCow<'a, str> = std::mem::transmute(right);
        left.cmp(right)
    }
}

How can we be sure that the unsafe usage is sound, though? Here we're using RCow only for a comparison; lifetimes don't even matter. Both 'a and 'b will live for at least as long as the function does, so it should be fine for a simple comparison operation.

Note, however, that you can't improve the ergonomics by creating a wrapper of RCow that uses mem::transmute internally. This is because invariant relationships are inherited. A simpler example would be &mut T, which is invariant over T. So if a type has a &mut T in one of its fields, then that type will also be invariant over T.

Here, SCow will also be invariant over 'a, because RCow is invariant over 'a:

struct SCow<'a>(RCow<'a, ()>);

We would have to hide RCow under a *const () and pointer-cast back and forth from it, but that sounds like an exceptionally bad idea.

You can find a playground link with the transmute approach in here. However, that would be a last resort hack that I would very much rather avoid. I really wouldn't want anyone to run into this issue again.

@rodrimati1992, do you think removing BorrowOwned would be viable for the library? Should I even give it a try or are you against it? This playground link showcases how using ToOwned instead of BorrowOwned would work -- it would basically be the same as Cow's implementation. We could use a custom trait such that Trait: ToOwned with further functionality if needed, but the point is that it can't have associated types.

Downsides:

It would be a backwards-incompatible change, since everyone would have to use RCow<RStr> instead of RCow<str>.

Upsides:

It's good for consistency; using RCow<str> where str isn't even FFI-safe always seemed confusing to me.
The whole implementation of RCow would be considerably simpler.
Changing from std to abi_stable is currently plain broken. That would fix it.

P.S. Sorry if this is overly verbose. I wanted to explain everything to make sure it's correct and so that my position is understood clearly. I will release a full writeup of this for https://nullderef.com/ whenever I find time because I find it to be a pretty intersting topic, but for now this should have enough details to get the issue moving forward.

rodrimati1992 · 2022-02-09T18:52:33Z

@rodrimati1992, do you think removing BorrowOwned would be viable for the library? Should I even give it a try or are you against it? This playground link showcases how using ToOwned instead of BorrowOwned would work -- it would basically be the same as Cow's implementation. We could use a custom trait such that Trait: ToOwned with further functionality if needed, but the point is that it can't have associated types.

Borrowed(&'a B) doesn't work for RStr, since RCow::from(rstr!("foo")) (impl<'a> From<RStr<'a>> for RCow<...) is supposed to work, but it would have to add a borrow from nowhere.

I'll mention that if you're redesigning RCow it'll have to have equivalents of:

Cow<'_, str>
Cow<'_, [T]>
Cow<'_, T> for all T: Sized types

marioortizmanero · 2022-02-17T11:51:51Z

After trying several new designs for RCow, I would say that the requirements make it an impossible task. We're trying to combine two cases:

An RCow where the Borrowed variant actually borrows (i.e., Borrowed(&'a B)). This is required for the general case of any T, just like how the Cow in the standard library works.
An RCow where the Borrowed variant does not borrow (i.e., Borrowed(B) and B: 'a). This would make it work for the case of RStr<'a> and RSlice<'a>, and should only be used for these.

Solution: Having two `RCow`s

As far as I know, the only way to fix this is to just have one RCow for RStr and RSlice's cases, and another one that can be generic over T.

pub mod regular_rcow {
   pub enum RCow<'a, T>
       where
           T: ToOwned
   {
       Borrowed(&'a T),
       Owned(<T as ToOwned>::Owned),
   }
}

pub mod sabi_rcow {
   pub enum RCow<B, O> {
       Borrowed(B),
       Owned(O),
   }

   pub type RCowStr<'a> = RCow<RStr<'a>, RString>;
   pub type RCowSlice<'a, T> = RCow<'a, RSlice<'a, T>, RVec<T>>;
}

Usage:

use sabi_rcow::RCowStr;
use regular_rcow::RCow;

fn test1<'a, 'b>(left: &RCowStr<'a>, right: &RCowStr<'b>) -> Ordering {
    left.cmp(right)
}
fn test2<'a, 'b>(left: &RCow<'a, u8>, right: &RCow<'b, u8>) -> Ordering {
    left.cmp(right)
}

Link to the playground that showcases how this would work

Regular `RCow`

The implementation and usage is just like the regular Cow. However, if the user wishes to have an RCow<'a, RStr<'a>>, they will have to use RCowStr<'a>, which is a type alias of the other version.

"Sabi" `RCow`

Meant to be used only for RStr, RSlice, and similars (not sure if I'm missing any).

Furthermore, this RCow should be used via the type alias RCowStr and RCowSlice only, as otherwise it would be possible to have strange combinations, such as RCow<u8, String>

This is kind of already a thing, see cervine::Cow. The only difference is that we don't actually borrow anything inside RCow. The lifetime is bound to the generic parameters instead.

Notes

Problem #1: how do we avoid footguns? Using RCow<'a, RStr<'a>> will just hold a reference to the RStr<'a> or RStr<'a> itself, so it's pointless. I guess just through documentation? Any other ideas?

Problem #2: this solution would be a breaking change for all users of RCow<'a, str>. Possibly in strange ways, too, because the only thing that would break would be that RCow<'a, str> isn't StableAbi anymore. Perhaps this would be better if RCow changed name or something? Maybe RCowAny (since we would also have RCowStr and RCowSlice)?

Other discarded ideas

Just using regular_rcow::RCow<'a, RStr<'a>> (which would imply the double borrow you mentioned). But it's impossible for two reasons:
```
// Impossible!
impl<'a> ToOwned for RStr<'a> {
	type Owned = RString;
	// ...
}
```
- ToOwned already has a blanket implementation for T: Clone [1]. RStr implements Clone.
- That would also require RString implementing Borrow<RStr<'a>>, and that doesn't make sense because of borrow's function signature. Its return type is &T [2], so the implementation could return a &'a RStr<'a> at most.
Having RStr be unsized (DST): that's impossible right now [3]. The only way to create an unsized type is to create it based on an already existing one (str, [T], dyn T).
Having &str be #[repr(C)]: that would depend on the Rust compiler, so we can't change it
Implementing RCow with a different approach such as beef's. Problem: that doesn't really fix the type association issue; beef::Cow also relies on ToOwned.

So, @rodrimati1992, do you think I should try to make a PR with the solution? Are you OK with these changes?

[1] https://doc.rust-lang.org/std/borrow/trait.ToOwned.html#impl-ToOwned-5
[2] https://doc.rust-lang.org/std/borrow/trait.Borrow.html
[3] https://doc.rust-lang.org/nomicon/exotic-sizes.html#dynamically-sized-types-dsts. Note the (Yes, custom DSTs are a largely half-baked feature for now.) at the end, and also this section: https://doc.rust-lang.org/nomicon/exotic-sizes.html#extern-types.

rodrimati1992 · 2022-02-18T04:41:19Z

Here's an alternate approach which takes the enum RCow<B, O> and uses it for the three usecases in one go:

Code

use abi_stable::std_types::{RSlice, RVec, RStr, RString};

use std::borrow::Borrow;
use std::cmp::{PartialEq, PartialOrd, Ord, Ordering};
use std::ops::Deref;

///////////////////////////////////////////////////////////////////////

trait QueryOwnedType: Deref {
    type Owned: Borrow<Self::Target>;
}

trait IntoOwned: Copy + QueryOwnedType {
    fn into_owned(self) -> Self::Owned;
}


impl<T> QueryOwnedType for &T {
    type Owned = T;
}

impl<T: Clone> IntoOwned for &T {
    fn into_owned(self) -> T {
        self.clone()
    }
}


impl QueryOwnedType for RStr<'_> {
    type Owned = RString;
}

impl IntoOwned for RStr<'_> {
    fn into_owned(self) -> RString {
        self.into()
    }
}


impl<T> QueryOwnedType for RSlice<'_, T> {
    type Owned = RVec<T>;
}

impl<T: Clone> IntoOwned for RSlice<'_, T> {
    fn into_owned(self) -> RVec<T> {
        self.to_rvec()
    }
}

///////////////////////////////////////////////////////////////////////

#[derive(Debug)]
enum RCow<B, O>{
    Borrowed(B),
    Owned(O),
}

type BCow<'a, T> = RCow<&'a T, T>;
type RCowStr<'a> = RCow<RStr<'a>, RString>;
type RCowSlice<'a, T> = RCow<RSlice<'a, T>, RVec<T>>;


impl<B: IntoOwned> RCow<B, B::Owned> {
    fn make_mut(&mut self) -> &mut B::Owned {
        match self {
            RCow::Borrowed(x) => {
                *self = RCow::Owned(x.into_owned());
                if let RCow::Owned(x) = self {
                    x
                } else {
                    unreachable!()
                }
            },
            RCow::Owned(x) => x,
        }
    }
}


impl<B> PartialEq for RCow<B, B::Owned> 
where
    B: QueryOwnedType,
    B::Target: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        **self == **other
    }
}

impl<B> Eq for RCow<B, B::Owned> 
where
    B: QueryOwnedType,
    B::Target: Eq,
{}

impl<B> PartialOrd for RCow<B, B::Owned> 
where
    B: QueryOwnedType,
    B::Target: PartialOrd,
{
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        (&**self).partial_cmp(&**other)
    }
}

impl<B> Ord for RCow<B, B::Owned> 
where
    B: QueryOwnedType,
    B::Target: Ord,
{
    fn cmp(&self, other: &Self) -> Ordering {
        (&**self).cmp(&**other)
    }
}

impl<B> Deref for RCow<B, B::Owned> 
where
    B: QueryOwnedType,
{
    type Target = B::Target;

    fn deref(&self) -> &Self::Target {
        match self {
            RCow::Borrowed(x) => x,
            RCow::Owned(x) => x.borrow(),
        }
    }
}


fn eq_rcow<'a, 'b, T>(left: &BCow<'_, T>, right: &BCow<'_, T>) -> bool 
where
    T: PartialEq
{
    RCow::eq(left, right)
}

fn cmp_rcow<'a, 'b, T>(left: &BCow<'a, T>, right: &BCow<'b, T>) -> Ordering
where
    T: Ord
{
    RCow::cmp(left, right)
}

fn eq_rcow_str<'a, 'b>(left: &RCowStr<'a>, right: &RCowStr<'b>) -> bool {
    RCow::eq(left, right)
}

fn cmp_rcow_str<'a, 'b>(left: &RCowStr<'a>, right: &RCowStr<'b>) -> Ordering {
    RCow::cmp(left, right)
}


fn eq_rcow_slice<'a, 'b, T>(left: &RCowSlice<'a, T>, right: &RCowSlice<'b, T>) -> bool 
where
    T: PartialEq
{
    RCow::eq(left, right)
}

fn cmp_rcow_slice<'a, 'b, T>(left: &RCowSlice<'a, T>, right: &RCowSlice<'b, T>) -> Ordering 
where
    T: Ord
{
    RCow::cmp(left, right)
}



fn main() {
    {
        let mut hi = RCowStr::Borrowed("hello".into());
        hi.make_mut().push_str(" world");
        println!("{hi:?}");
    }

    {
        let value = 0u8;
        let left = BCow::Borrowed(&value);
        let right = BCow::Borrowed(&value);
        dbg!(cmp_rcow(&left, &right));
    }
    {
        let rstr = RStr::from_str("abc");
        let left = RCowStr::Borrowed(rstr.clone());
        let right = RCowStr::Borrowed(rstr.clone());
        dbg!(cmp_rcow_str(&left, &right));
    }
}

marioortizmanero · 2022-02-18T12:51:14Z

Nice! So if ToOwned doesn't work we can always make our own. The only issue I can find here is that T in RCow requires Deref now, though that makes sense for borrowed/owned types. I assume you're ok with that, so shall I make a PR replacing RCow?

Can we just skip the O generic parameter in RCow and always use B::Owned there? I feel like that is unnecessary now, and we can remove the BCow type alias.
Can we combine IntoOwned and QueryOwnedType into a single trait?

marioortizmanero · 2022-02-18T18:54:55Z

Ugh... After the PR that should've fixed this, I've ran into other (worse) errors.

One of them is that RVec<T> is fundamentally invariant. It used to have a *mut T, which made it invariant over T [1]. So I've changed it to NonNull<T> and thought it'd be fixed. But turns out that the vtable it holds also makes RVec<T> invariant, for two reasons:

The vtable has function pointers with RVec<T> as a parameter, which make it contravariant over T. But since other parts of RVec<T> are covariant, then Rust defaults to RVec<T> being invariant (i.e., in case of conflict, it's just invariant; it can't be both).
The function pointers have specifically &mut RVec<T> as their parameter, which makes it invariant anyway for being mutable.

In summary, I am going to lose my sanity, honestly. Is it possible to remove the vtable from RVec<T> so that it's covariant over T?

Playground with proof: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=57e81d749461030ab5f590368253f6f2

Also, here's the test I'm using to prove that the lifetimes work fine, for reference

use abi_stable::std_types::{RCowStr, RHashMap, RSlice, RStr, RString, RVec, Tuple2};
use std::{borrow::Cow, collections::HashMap, cmp::Ordering};

fn cmp_string<'a, 'b>(left: &String, right: &String) -> Ordering {
    left.cmp(right)
}
fn cmp_rstring<'a, 'b>(left: &RString, right: &RString) -> Ordering {
    left.cmp(right)
}
fn cmp_string2<'a, 'b>(left: &String, right: &String) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rstring2<'a, 'b>(left: &RString, right: &RString) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_str<'a, 'b>(left: &'a str, right: &'b str) -> Ordering {
    left.cmp(right)
}
fn cmp_rstr<'a, 'b>(left: &RStr<'a>, right: &RStr<'b>) -> Ordering {
    left.cmp(right)
}
fn cmp_str2<'a, 'b>(left: &'a str, right: &'b str) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rstr2<'a, 'b>(left: &RStr<'a>, right: &RStr<'b>) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_vec<'a, 'b>(left: &Vec<&'a str>, right: &Vec<&'b str>) -> Ordering {
    left.cmp(right)
}
fn cmp_rvec<'a, 'b>(left: &RVec<&'a str>, right: &RVec<&'b str>) -> Ordering {
    left.cmp(right)
}
fn cmp_vec2<'a, 'b>(left: &Vec<&'a str>, right: &Vec<&'b str>) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rvec2<'a, 'b>(left: &RVec<&'a str>, right: &RVec<&'b str>) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_tpl<'a, 'b>(left: &(&'a str, u8), right: &(&'b str, u8)) -> Ordering {
    left.cmp(right)
}
fn cmp_rtpl<'a, 'b>(left: &Tuple2<&'a str, u8>, right: &Tuple2<&'b str, u8>) -> Ordering {
    left.cmp(right)
}
fn cmp_tpl2<'a, 'b>(left: &(&'a str, u8), right: &(&'b str, u8)) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rtpl2<'a, 'b>(left: &Tuple2<&'a str, u8>, right: &Tuple2<&'b str, u8>) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_slice<'a, 'b>(left: &[&'a str], right: &[&'b str]) -> Ordering {
    left.cmp(right)
}
fn cmp_rslice<'a, 'b>(left: &RSlice<&'a str>, right: &RSlice<&'b str>) -> Ordering {
    left.cmp(right)
}
fn cmp_slice2<'a, 'b>(left: &[&'a str], right: &[&'b str]) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rslice2<'a, 'b>(left: &RSlice<&'a str>, right: &RSlice<&'b str>) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_cowstr<'a, 'b>(left: &Cow<'a, str>, right: &Cow<'b, str>) -> Ordering {
    left.cmp(right)
}
fn cmp_rcowstr<'a, 'b>(left: &RCowStr<'a>, right: &RCowStr<'b>) -> Ordering {
    left.cmp(right)
}
fn cmp_cowstr2<'a, 'b>(left: &Cow<'a, str>, right: &Cow<'b, str>) -> Option<Ordering> {
    left.partial_cmp(right)
}
fn cmp_rcowstr2<'a, 'b>(left: &RCowStr<'a>, right: &RCowStr<'b>) -> Option<Ordering> {
    left.partial_cmp(right)
}

fn cmp_hashmap<'a, 'b>(left: &HashMap<u8, &'a str>, right: &HashMap<u8, &'b str>) -> bool {
    left.eq(right)
}
fn cmp_rhashmap<'a, 'b>(left: &RHashMap<u8, &'a str>, right: &RHashMap<u8, &'b str>) -> bool {
    left.eq(right)
}

[1] https://doc.rust-lang.org/nomicon/subtyping.html#variance

Licenser · 2022-02-18T22:57:08Z

I wonder, since we're deep in unsafe terretory, and the vtable is internally managed anyway, would something like this work:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2c8154fd9d32792d2e14f90e14628030

sneaky, dirty but perhaps possible?

rodrimati1992 · 2022-02-19T01:13:35Z

Can we just skip the O generic parameter in RCow and always use B::Owned there? I feel like that is unnecessary now

From my experimentation, it causes the same kind of lifetime issues to do

enum RCow<B: QueryOwnedType>{
    Borrowed(B),
    Owned(B::Owned),
}

as before the redesign. So removing the O type parameter doesn't work AFAICT.

Can we combine IntoOwned and QueryOwnedType into a single trait?

Yes. I split it to remove T: Clone bounds, but I realize that it's what Cow requires already, so it'd not be any more of a burden to have it as a single IntoOwned trait.

Can we remove the BCow type alias.

I could remove it, but then it'll be RCow<&'a T, T>.

I wonder, since we're deep in unsafe terretory, and the vtable is internally managed anyway, would something like this work:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2c8154fd9d32792d2e14f90e14628030

Yes, I'll make it work, it'll be bundled along with the other breaking changes to make more lifetimes covariant (I don't have a planned release date for that 0.11.0 release).

marioortizmanero · 2022-02-19T13:57:22Z

From my experimentation, it causes the same kind of lifetime issues to do

Yup, you're completely right. I've changed it back to RCow<B, O> and it works fine now.

I could remove it, but then it'll be RCow<&'a T, T>.

Yeah if we have two generic parameters then it makes sense to have the alias.

Yes, I'll make it work, it'll be bundled along with the other breaking changes to make more lifetimes covariant (I don't have a planned release date for that 0.11.0 release).

Do you mean you want to do it yourself? You can make a PR over #80, any help is very much appreciated :)

marioortizmanero · 2022-02-23T11:41:11Z

I've opened a new issue with the tasks that must be done: #81. This one is quite filled with discussion about how we figured out that the issue was variance, and it will be easier to track the progress in there. You can close this if you like, @rodrimati1992.

Fixing the lifetime issues (WRT RCow) in #75 Changed how `RCow` is represented, requiring uses of the `RCow` type to be updated. Added `RCowVal`, `RCowStr`, and `RCowSlice` type aliases to make `RCow` usable. Updated uses of `RCow` in `abi_stable` to make it compile, the repository will be fixed in a latter commit. Updated RCow tests. Added tests for comparison traits, conversion traits. Added `IntoOwned` trait. Made the comparison traits accept RCows with different type arguments. Added these conversion impls: - `From<&'a RVec<T>> for RCowSlice<'a, T>` - `From<&'a Vec<T>> for RCowSlice<'a, T>` Now the conversion impls between `Cow` and `RCow` are non-trivial, using the new `RCowCompatibleRef` trait to simplify some bounds. Now `RCow` only implements `IntoReprRust`/`AsRef`/`Borrow` for `RCowVal`, `RCowSlice`, and `RCowStr`. Removed `BorrowOwned` trait.

marioortizmanero · 2022-02-24T00:11:46Z

This has finally been fixed and can be closed.

* Back to attempt with single value * Go back to known_key stub approach * Update to new `RCow` * Lots of fixes and cleaning up See rodrimati1992/abi_stable_crates#75

* Back to attempt with single value * Go back to known_key stub approach * Update to new `RCow` * Lots of fixes and cleaning up * Fix metronome plugin See rodrimati1992/abi_stable_crates#75

Fixing the lifetime issues (WRT RCow) in #75 Changed how `RCow` is represented, requiring uses of the `RCow` type to be updated. Added `RCowVal`, `RCowStr`, and `RCowSlice` type aliases to make `RCow` usable. Updated uses of `RCow` in `abi_stable` to make it compile, the repository will be fixed in a latter commit. Updated RCow tests. Added tests for comparison traits, conversion traits. Added `IntoOwned` trait. Made the comparison traits accept RCows with different type arguments. Added these conversion impls: - `From<&'a RVec<T>> for RCowSlice<'a, T>` - `From<&'a Vec<T>> for RCowSlice<'a, T>` Now the conversion impls between `Cow` and `RCow` are non-trivial, using the new `RCowCompatibleRef` trait to simplify some bounds. Now `RCow` only implements `IntoReprRust`/`AsRef`/`Borrow` for `RCowVal`, `RCowSlice`, and `RCowStr`. Removed `BorrowOwned` trait.

marioortizmanero mentioned this issue Feb 1, 2022

Fix R* lifetimes #76

Closed

marioortizmanero mentioned this issue Feb 9, 2022

Subtyping and Variance - Trait variance not covered rust-lang/nomicon#338

Open

marioortizmanero mentioned this issue Feb 15, 2022

Separate the FFI-safe types from the rest of the crate #79

Open

marioortizmanero mentioned this issue Feb 18, 2022

Fix R* lifetimes via RCow redesign #80

Closed

marioortizmanero mentioned this issue Feb 23, 2022

Making abi_stable's types covariant #81

Closed

8 tasks

rodrimati1992 closed this as completed Feb 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lifetimes with R* types break compared to non R* types #75

lifetimes with R* types break compared to non R* types #75

Licenser commented Jan 31, 2022

marioortizmanero commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 1, 2022

marioortizmanero commented Feb 1, 2022 •

edited

Loading

rodrimati1992 commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 2, 2022 •

edited

Loading

marioortizmanero commented Feb 9, 2022 •

edited

Loading

rodrimati1992 commented Feb 9, 2022 •

edited

Loading

marioortizmanero commented Feb 17, 2022 •

edited

Loading

rodrimati1992 commented Feb 18, 2022 •

edited

Loading

marioortizmanero commented Feb 18, 2022 •

edited

Loading

marioortizmanero commented Feb 18, 2022 •

edited

Loading

Licenser commented Feb 18, 2022

rodrimati1992 commented Feb 19, 2022 •

edited

Loading

marioortizmanero commented Feb 19, 2022 •

edited

Loading

marioortizmanero commented Feb 23, 2022

marioortizmanero commented Feb 24, 2022

lifetimes with R* types break compared to non R* types #75

lifetimes with R* types break compared to non R* types #75

Comments

Licenser commented Jan 31, 2022

marioortizmanero commented Feb 1, 2022 • edited Loading

marioortizmanero commented Feb 1, 2022

marioortizmanero commented Feb 1, 2022 • edited Loading

rodrimati1992 commented Feb 1, 2022 • edited Loading

marioortizmanero commented Feb 1, 2022 • edited Loading

marioortizmanero commented Feb 2, 2022 • edited Loading

marioortizmanero commented Feb 9, 2022 • edited Loading

rodrimati1992 commented Feb 9, 2022 • edited Loading

marioortizmanero commented Feb 17, 2022 • edited Loading

Solution: Having two RCows

Regular RCow

"Sabi" RCow

Notes

Other discarded ideas

rodrimati1992 commented Feb 18, 2022 • edited Loading

marioortizmanero commented Feb 18, 2022 • edited Loading

marioortizmanero commented Feb 18, 2022 • edited Loading

Licenser commented Feb 18, 2022

rodrimati1992 commented Feb 19, 2022 • edited Loading

marioortizmanero commented Feb 19, 2022 • edited Loading

marioortizmanero commented Feb 23, 2022

marioortizmanero commented Feb 24, 2022

marioortizmanero commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 1, 2022 •

edited

Loading

rodrimati1992 commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 1, 2022 •

edited

Loading

marioortizmanero commented Feb 2, 2022 •

edited

Loading

marioortizmanero commented Feb 9, 2022 •

edited

Loading

rodrimati1992 commented Feb 9, 2022 •

edited

Loading

marioortizmanero commented Feb 17, 2022 •

edited

Loading

Solution: Having two `RCow`s

Regular `RCow`

"Sabi" `RCow`

rodrimati1992 commented Feb 18, 2022 •

edited

Loading

marioortizmanero commented Feb 18, 2022 •

edited

Loading

marioortizmanero commented Feb 18, 2022 •

edited

Loading

rodrimati1992 commented Feb 19, 2022 •

edited

Loading

marioortizmanero commented Feb 19, 2022 •

edited

Loading