Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add String::leak #109

Closed
finnbear opened this issue Sep 18, 2022 · 1 comment
Closed

Add String::leak #109

finnbear opened this issue Sep 18, 2022 · 1 comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@finnbear
Copy link

finnbear commented Sep 18, 2022

Proposal

Problem statement

When working in a resource-constrained environment (e.g. Webassembly), it can be useful to leak a small number of String's to avoid cloning later on.

However, the existing Box::leak(string.into_boxed_str()) may unecessarily reallocate the String, which is presumably why
Vec::<u8>::leak exists.

Motivation, use-cases

For example, the following code leaks a String without reallocation.

// Simulate creation of an <svg>
let string = String::with_capacity(1000);
string.push_str("<svg>");
string.push_str("</svg>");

// New API (never reallocates)
let leaked: &'static mut str = string.leak();

// Hand out to a global memory location, such that consumers of <svg> can access it without cloning.
svgs.write().insert("test", &*leaked);

Solution sketches

impl String {
    pub fn leak<'a>(self) -> &'a mut str {
        let me = self.into_bytes().leak();
        // Safety: Bytes from a [`String`] are valid utf8.
        unsafe { std::str::from_utf8_unchecked_mut(me) }
    }	
}

*note: It may be necessary to bound the allocator's lifetime appropriately, if and when this lands.

Links and related work

As previously emphasized, this change is an extension of Vec::<u8>::leak.

Right now, StackOverflow answers suggest using
Box::leak(string.into_boxed_str()), which shrinks the string to fit, possibly causing unnecessary reallocation.

I previously submitted this as an RFC but that was deemed overkill.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@finnbear finnbear added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Sep 18, 2022
@dtolnay
Copy link
Member

dtolnay commented Oct 11, 2022

Seconded. I am on board with the API as you've proposed it. :) Please go ahead and open a standard library PR, and a library tracking issue: https://github.com/rust-lang/rust/issues/new/choose

@dtolnay dtolnay closed this as completed Oct 11, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 22, 2022
…htriplett

(rust-lang#102929) Implement `String::leak` (attempt 2)

Implementation of `String::leak` (rust-lang#102929)

ACP: rust-lang/libs-team#109

Supersedes rust-lang#102941 (see previous reviews there)

`@rustbot` label +T-libs-api -T-libs
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 22, 2022
…htriplett

(rust-lang#102929) Implement `String::leak` (attempt 2)

Implementation of `String::leak` (rust-lang#102929)

ACP: rust-lang/libs-team#109

Supersedes rust-lang#102941 (see previous reviews there)

``@rustbot`` label +T-libs-api -T-libs
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 22, 2022
…htriplett

(rust-lang#102929) Implement `String::leak` (attempt 2)

Implementation of `String::leak` (rust-lang#102929)

ACP: rust-lang/libs-team#109

Supersedes rust-lang#102941 (see previous reviews there)

```@rustbot``` label +T-libs-api -T-libs
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
(#102929) Implement `String::leak` (attempt 2)

Implementation of `String::leak` (#102929)

ACP: rust-lang/libs-team#109

Supersedes #102941 (see previous reviews there)

```@rustbot``` label +T-libs-api -T-libs
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 27, 2023
…etime, r=Amanieu

Use an unbounded lifetime in `String::leak`.

Using `'a` instead of `'static` is predicted to make the process of making `String` generic over an allocator easier/less of a breaking change.

See:
- rust-lang#109814 (comment)
- rust-lang#109814 (comment)

ACP: rust-lang/libs-team#109
@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label May 30, 2023
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
…Amanieu

Use an unbounded lifetime in `String::leak`.

Using `'a` instead of `'static` is predicted to make the process of making `String` generic over an allocator easier/less of a breaking change.

See:
- rust-lang/rust#109814 (comment)
- rust-lang/rust#109814 (comment)

ACP: rust-lang/libs-team#109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

2 participants