Update once_cell 'get_or_init' to 'get_or_init_with'

[tgross35]

Potentially blocks #105587

I hate myself for bikeshedding on a stabilization PR I care about, but I think get_or_init_with might be a better name for get_or_init.

The main reason is it might make sense in the future to re-add a get_or_init that takes a value rather than a closure. I kind of doubt it would ever get added (unless we elect to do it now) as it's just easy sugar for .get_or_init_with(|| val). However, unless we pick a _with name for the FnOnce version now, we wouldn't be able to add it later and keep a consistent naming scheme.

Other points are that the near cousin RefCell has replace(&self, T) and replace_with<F: FnOnce...>(&self, f: F), so changing the name would make this consistent. And the _with naming pattern is well established anyway, as I mention here #105587 (comment)

Downsides of changing are, of course, potential confusion for anyone using the crate, and the slightly longer name.

I'm not tied to the get_or_init portion specifically, and matklad mentioned some other options here #74465 (comment).

cc @matklad

[rustbot]

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[tgross35]

Two things also regarding get_or_try_init/get_or_try_init_with:

1. get_or_try_init may be fine since I don't think there could ever be a get_or_try_init(val)/get_or_try_init_with(|| func()) pair
2. These are the only functions in std that have try somewhere in the middle, compared to 110 functions that start with try. I think the current name makes sense for the intuition of what it's doing (either getting the values or trying to initialize them), but moving the try to the beginning would definitely be more consistent. (searches: rg 'pub[^(].*fn.*_try.*' --type rust library/ --stats rg 'pub[^(].*fn.* try_.*' --type rust library/ --stats)

Less pressure to think about this since they're no longer in #74465

[tgross35]

@rustbot modify labels: +T-libs-api -T-rustdoc -T-libs -T-compiler

r? m-ou-se

(just since you're the assignee on the stabilization PR)

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[tgross35]

Sorry for the noise MIR/compiler/clippy team

[bors]

☔ The latest upstream changes (presumably #106975) made this pull request unmergeable. Please resolve the merge conflicts.

[dtolnay]

None of the justifications described above are compelling to me and I would prefer to keep get_or_init.

Particularly this main reason:

The main reason is it might make sense in the future to re-add a get_or_init that takes a value rather than a closure. I kind of doubt it would ever get added (unless we elect to do it now) as it's just easy sugar for .get_or_init_with(|| val). However, unless we pick a _with name for the FnOnce version now, we wouldn't be able to add it later and keep a consistent naming scheme.

I'd be happy to rule this out now. That's effectively what the team has already done by approving the FCP to stabilize get_or_init.

I can't imagine that initializing with a value expression instead of a closure would be a common enough use case of get_or_init to justify an extra method dedicated to making it marginally more convenient. Comparing the following scenarios:

// 5% use case?
once_cell.get_or_init(|| val)
once_cell.get_or_init(val)  // proposed change: -3 characters

// 95% use case?
once_cell.get_or_init(|| ...)
once_cell.get_or_init_with(|| ...)  // proposed change: +5 characters

I don't really see this coming out in favor of a rename. The API is more convenient much more of the time prior to this PR.

[SUPERCILEX]

Does the libs team have a doc that outlines how to pick function names? If not, it could be valuable to write down the thought process that leads to whatever decision is taken here so it can be referred to in the future.

---

I personally don't see the get_or_init(val) use case either (hadn't even considered it), but I like get_or_init_with for consistency and clarity reasons. It "reads better" in the sense that you immediately know what the closure will be used for. Then again, on the clarify front a functional programmer would say the name is irrelevant anyway and you should be looking at the signature, so the _with is implied by the fact that the function takes in a closure. I could see it going either way, but I think I prefer get_or_init_with.

Sidenote: I don't see "+5 characters" as a good argument. Readability is way more important than writability.

[m-ou-se]

Un-approving this for now, because we can't stabilize this unless all libs-api members are on board with it. ^^

(I personally don't feel strongly about this. I can see how _with can be a bit more consistent, but get_or_init also seems fine to me.)

[dtolnay]

3 people have said the word consistency, but I don't understand a consistency argument for this PR either. "Put _with in the name of functions that take a closure" is not an idiom in the standard library, and would not be a useful idiom. "Pick names that differentiate among similar functions" is an idiom. That's why there would be no point naming result.map_with(...), iterator.filter_with(...), thread::spawn_with(...) etc.

[m-ou-se]

I agree with that. (See also bool::then. ^^)

I could see there is some value in treating OnceLock as shared Option, basically, in which case we're talking about consistency with Option::get_or_insert[_with] specifically. Personally, though, I have a very slight preference for just get_or_init (without _with), but I'd be happy with either.

(From the discusson on #105587 I got the impression that get_or_init_with might be generally preferred, but looks like that's not the case. :) )

[tgross35]

The reason I brought this up was because it hadn't been discussed much, as @matklad pointed out in #105587 (right at the end of FCP). The final place that naming discussion ever came up is here #74465 (comment) where I assumed the 5 upvotes were showing mild +1 for get_or_init_with.

Option::get_or_insert_with and RefCell::replace_with are two similar container types that do have separate _x and _x_with methods - as opposed to the combinator and spawn examples, which are quite different cases. I was unsure what the motivation was for having those separate, but figured it was at least worth discussing whether that would be desired here, just since it hadn't been.

All in all I'm quite OK closing this now that there's been some discussion, if there isn't strong motivation for the change (unless @BurntSushi has anything to say as the third RFC approver). If this is the case, then I think the stabilization PR is ready for merge.

[SUPERCILEX]

Looking at the other stdlib APIs that end in _with, it does seem like the only reason they exist is because there's a non-with method available.

Pick names that differentiate among similar functions

So then this makes sense for keeping it get_or_init.

[tgross35]

Looking at the other stdlib APIs that end in _with, it does seem like the only reason they exist is because there's a non-with method available.

Which just hadn't been discussed before - hence bringing it up here :)

[BurntSushi]

I like get_or_init, but don't feel strongly. So yeah, I'd say close this out and let's get this thing stabilized!

[matklad]

That's effectively what the team has already done by approving the FCP to stabilize get_or_init.

As a procedural observation, naming was (and still is) an unchecked [ ] concern on the corresponding tracking issue without resolution more explicit than “let’s roll with the historical option”, so, for an outside observer, the two worlds looked equivalently:

• the team made an explicit decision
• the team missed an open question

[tgross35]

Works for me :) so #105587 is merge ready.

The tracking issue #74465 could probably be updated to link to this for the naming checkbox, as @matklad pointed out