Revise documentation of NumBuffer and format_into(). - #162158
Conversation
|
Some changes occurred in integer formatting cc @tgross35 |
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
Thanks, this is a good improvement, but the “without heap allocation” part is misleading.
r? me @rustbot author
| /// `buf` as storage for the returned string slice. | ||
| /// | ||
| /// This can be used instead of ordinary [`Display`][fmt::Display] formatting to | ||
| /// format numbers without any heap allocations. |
There was a problem hiding this comment.
fmt machinery can be used without heap allocation, e.g., writing to any io::Write impl backed by a stack buffer (which won’t give you a &str for free, but can be combined with str::from_utf8[_unchecked]), or using third party types like ArrayString (which impls fmt::Write and trivially gives you a &str). Maybe just motivate it more generically as “more efficient” without going into details?
There was a problem hiding this comment.
You’re right about heap allocation, but I don’t want to say just “this can format numbers more efficiently” because that leads to misconceptions like “Display is gratuitously inefficient”. There should be some qualification that says which situations this is worth doing in. The exact internal difference is that format_into() avoids the dynamic dispatch inherent in fmt, and that’s it, right? I’ve revised the paragraph to:
This method can be used to convert integers to strings without involving the dynamic dispatch that using [
Display] would. This may be more efficient in situations where [fmt] is not otherwise used.
|
Reminder, once the PR becomes ready for a review, use |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
This comment has been minimized.
This comment has been minimized.
The previous documentation was, in my opinion, overly focused on the implementation of these items, rather than how and when to use them. This new documentation: * Begins the `format_into()` documentation with what it does when called. * Specifies that `format_into()` is a specialized alternative to the more commonly used `Display`. * Specifies that `NumBuffer` cannot be read or otherwise used by itself.
|
@bors r+ rollup |
…uwer Rollup of 4 pull requests Successful merges: - #162032 (remove more trivial regions in evaluate_added_goals_and_make_canonical_response) - #161607 (Add #[track_caller] to signed integer div/rem methods) - #162339 (Revert "bootstrap: Enable rustdoc mergeable CCI for std and internal docs #161716") - #162158 (Revise documentation of `NumBuffer` and `format_into()`.)
Rollup merge of #162158 - kpreid:numbuffer, r=hanna-kruppe Revise documentation of `NumBuffer` and `format_into()`. The previous documentation was, in my opinion, overly focused on the implementation of these items, rather than how and when to use them. In particular, this new documentation: * Begins the `format_into()` documentation with what it does when called. * Specifies that `format_into()` is a specialized alternative to the more commonly used `Display`. * Specifies that `NumBuffer` cannot be read or otherwise used by itself. @rustbot label +A-docs
The previous documentation was, in my opinion, overly focused on the implementation of these items, rather than how and when to use them. In particular, this new documentation:
format_into()documentation with what it does when called.format_into()is a specialized alternative to the more commonly usedDisplay.NumBuffercannot be read or otherwise used by itself.@rustbot label +A-docs