Rust: Tracking issue for `Vec::leak`

Created on 28 Jun 2019  Â·  20Comments  Â·  Source: rust-lang/rust

Similar to Box::leak, this function would allow leaking a Vec and getting back an &'a mut [T].

A-collections B-unstable C-tracking-issue Libs-Tracked T-libs

Most helpful comment

@Lucretiel the lifetime parameter is necessary in case T has non-'static lifetimes, e.g. if it's a reference (playground).

Box::leak has a lifetime parameter for the same reason.

All 20 comments

Box::leak is an associated function instead of a method to avoid shadowing a possible Foo::leak method in Box<Foo>, normally accessible through auto-deref.

This is less relevant to Vec::leak since Vec<T> dereferences to [T]. There is no [T]::leak method in the standard library, so making Vec::leak a method would only shadow such a method if it’s defined in a user trait.

@cramertj Do you think it should still be an associated function?

I don't have any particularly strong opinion. .leak() seems pretty clear to me, and users can always call it as Vec::leak if they prefer.

Is there any reason to not have this method return an &'static mut [T]? It simplifies the code (no generic) and Rust can coerce the lifetime freely.

I find weird that there is a Vec::leak, but not a String::leak, PathBuf::leak, OsString::leak or CString::leak.

IMHO, if Vec::leak exists, then any standard type that owns data and has a into_boxed_whatever member function should be able to leak too.

IMHO, if Vec::leak exists, then any standard type that owns data and has a into_boxed_whatever member function should be able to leak too.

Seems fine to me. Sadly, I don't think there's a trait that covers these, so they'll all have to be added independently.

Sadly, I don't think there's a trait that covers these, so they'll all have to be added independently.

I don't think the lack of trait is a problem at all. For example I would expect any standard type that has a len() function to have an is_empty() too, no trait involved. It's like an expected pattern.

@Lucretiel the lifetime parameter is necessary in case T has non-'static lifetimes, e.g. if it's a reference (playground).

Box::leak has a lifetime parameter for the same reason.

What do people think about changing the argument type to be more generic:

pub fn leak<'a>(vec: impl Into<Vec<T>>) -> &'a mut [T]
where
    T: 'a, 

I'm thinking about adding a similar String::leak and it'd be nice to do String::leak("hello").

If we were to change it in this way, wouldn't it be preferable to use the Borrow machinery so that we can leak Vec<T> -> &[T], String -> &str, etc?

I’d prefer to go in the opposite direction and change Vec::leak to be a proper method that takes a self parameter instead of an associated function with a vec: Self parameter. Any conversion can be a separate (chained) call.

Box::<T>::leak (like Box::into_raw etc) is an associated function rather than a method in order to avoid shadowing a potential T::leak method for some T types. But that does not apply to Vec which does not have Deref to a fully generic target type.

a proper method that takes a self parameter instead of an associated function

You and I have had this discussion for other types so I won’t restate my previous positions against this.

Any conversion can be a separate (chained) call

Vec::leak itself doesn’t need to exist and can be a separate (chained) call — vec -> boxed slice -> box::leak

use the Borrow machinery

I’m not following how this would work; would you mind expanding a bit?

So, your proposal is that we have leak be generic over Into<Vec<T>>, with the motivating example being that you can leak a String. However, my argument is that it's surprising for a String to leak into a &[u8], and that instead the Borrow trait (which universally genericizes & associates between owned and borrowed versions of things (eg, Box<T> -> &T, Vec<T> -> &[T], String -> &str) could be used instead.

Though, now that I'm thinking about it, I think it also does T->&T, so it may actually be too abstract for this purpose.

@shepmaster I’m sorry, I don’t remember the specifics so if you have a link to those discussions I wouldn’t mind.

By "chained" I was referring specifically to method chaining syntax .foo().bar(), where parens do not need to be nested. In this case, .into::<Vec<_>>().leak().

with the motivating example being that you can leak a String

Ah, sorry, that's not what I meant. I mean to add another method: String::leak in the same vein as Vec::leak and Box::leak.

impl String {
    pub fn leak(s: impl Into<String>) -> &'a mut str {}
}

Ah, yes, I definitely can support that.

But wait, so- that means (for example) that something like Vec::leak("Hello".to_string()) would be valid? I'm not sure I love that; it seems like it'd be more confusing than other cases of Into<Self> in method definitions. My expectation is to "leak a Vec", and I'd argue it's reasonable in this case for the user to manage getting a Vec they want to leak in the first place.

You and I have had this [self vs associated method] discussion for other types so I won’t restate my previous positions against this.

Additionally, can I ask you to elaborate on this a bit? It makes perfect sense to me for smart-pointer types like Box, Arc, etc, but I'm not sure I see the argument for Vec::leak or String::leak.

Additionally, can I ask you to elaborate on this a bit?

I'd rather not as I've always lost that particular argument. No reason to believe that this time will be any different.

I’ve proposed FCP to stabilize with a signature change at https://github.com/rust-lang/rust/pull/74605

Was this page helpful?
0 / 5 - 0 ratings