Skip to content

Tracking Issue for OnceCell/Lock::try_insert() #116693

Open
@daxpedda

Description

@daxpedda
Contributor

Feature gate: #![feature(once_cell_try_insert)]

This is a tracking issue for OnceCell::try_insert() and OnceLock::try_insert().

This adds a method similarly to OnceCell/Lock::set() but returns a reference at the same time. This is also similar to OnceCell/Lock::get_or_init() but the return value also tells you if the value was actually inserted or if the OnceCell/Lock was already occupied.

Public API

impl<T> OnceCell<T> {
    pub fn try_insert(&self, value: T) -> Result<&T, (&T, T)>;
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

Activity

added
C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on Oct 13, 2023
added a commit that references this issue on Oct 14, 2023

Rollup merge of rust-lang#116540 - daxpedda:once-cell-lock-try-insert…

fcd75cc
added a commit that references this issue on Oct 15, 2023
ChayimFriedman2

ChayimFriedman2 commented on Jan 3, 2024

@ChayimFriedman2
Contributor

Should the return type be Result<&T, (&T, T)> or (&T, Option<T>) or (&T, Result<T, ()>)? It is easier to use the (&T, Option<T>) version but it does not encode the fact that there was a failure to insert ((&T, Result<T, ()>) encodes this better, but still not as good as Result<&T, (&T, T)>). On the other hand, the differences are not that significant since one is likely to match or the result of try_insert() anyway, since for try_insert().unwrap() we have the better set().unwrap().

tisonkun

tisonkun commented on Feb 26, 2024

@tisonkun
Contributor

I'm working on #121641 and wonder if we should add &mut both in the receiver and the result. Generally insert is a mutation and we can get a mut ref for the result if succeed.

Zollerboy1

Zollerboy1 commented on Jun 16, 2024

@Zollerboy1

Is there a reason, why this takes a value directly instead of an FnOnce like get_or_init and friends do?

I would like to use this in a case where I have to know after a get_or_init if the call inserted a value or not, but with the current design it seems that I have to check with get first to avoid creating a value that's not actually needed.

daxpedda

daxpedda commented on Jun 16, 2024

@daxpedda
ContributorAuthor

I'm unsure how I missed these comments but here goes:

Should the return type be Result<&T, (&T, T)> or (&T, Option<T>) or (&T, Result<T, ()>)? It is easier to use the (&T, Option<T>) version but it does not encode the fact that there was a failure to insert ((&T, Result<T, ()>) encodes this better, but still not as good as Result<&T, (&T, T)>). On the other hand, the differences are not that significant since one is likely to match or the result of try_insert() anyway, since for try_insert().unwrap() we have the better set().unwrap().

Given that all these options fulfill the requirements, personally I'm still in favor of Result<&T, (&T, T)> because it seems to be the easiest to use.

I'm working on #121641 and wonder if we should add &mut both in the receiver and the result. Generally insert is a mutation and we can get a mut ref for the result if succeed.

I'm assuming you mean adding new &mut APIs, because altering the ones proposed here would defeat its purpose.

The only use case I know of is only valid if the value is shared, otherwise get_or_try_init() would cover it. So I'm unsure if a &mut API addition would bring any value to the table in this case, seeing that get_mut_or_try_init() exists.

Is there a reason, why this takes a value directly instead of an FnOnce like get_or_init and friends do?

I would like to use this in a case where I have to know after a get_or_init if the call inserted a value or not, but with the current design it seems that I have to check with get first to avoid creating a value that's not actually needed.

That sounds quite reasonable to me, I will make a PR proposing this change when I get to it.

daxpedda

daxpedda commented on Jun 17, 2024

@daxpedda
ContributorAuthor

Is there a reason, why this takes a value directly instead of an FnOnce like get_or_init and friends do?
I would like to use this in a case where I have to know after a get_or_init if the call inserted a value or not, but with the current design it seems that I have to check with get first to avoid creating a value that's not actually needed.

That sounds quite reasonable to me, I will make a PR proposing this change when I get to it.

While implementing it I realized that this is a bit different.

This API is more similar to set(), which takes a value as well. Taking a function would have different tradeoffs, e.g. any values owned by this function would be lost, users would have to use workarounds like this:

let mut value = Some(value);

match self.try_insert(|| value.take().unwrap()) {
    Ok(value) => do_your_thing(),
    Err(old_value) => {
        let new_value = value.unwrap();
        do_your_other_thing();
    }
}

So I think this would need another API addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @daxpedda@tisonkun@Zollerboy1@ChayimFriedman2

        Issue actions

          Tracking Issue for `OnceCell/Lock::try_insert()` · Issue #116693 · rust-lang/rust