Add docs about performance and Iterator::map to [T; N]::map
#87609
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
It could also be added a warning when the programmer attempts to use map on a sufficiently large array. |
|
@bors r+ rollup |
|
📌 Commit 5cc7702 has been approved by |
bors
added
S-waiting-on-bors
That would require us to set some kind of threshold, which is hard. And there are cases where |
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Jul 30, 2021
…ocs, r=m-ou-se Add docs about performance and `Iterator::map` to `[T; N]::map` This suboptimal code gen for some usages of array::map got a bit of attention by multiple people throughout the community. Some cases: - rust-lang#75243 (comment) - rust-lang#75243 (comment) - https://www.reddit.com/r/rust/comments/oeqqf7/unexpected_high_stack_usage/ My *guess* is that this gets the attention it gets because in JavaScript (and potentially other languages), a `map` function on arrays is very commonly used since in those languages, arrays basically take the role of Rust's iterator. I considered explicitly naming JavaScript in the first paragraph I added, but I couldn't find precedence of mentioning other languages in standard library doc, so I didn't add it. When array::map was stabilized, we still wanted to add docs, but that somehow did not happen in time. So here we are. Not sure if this sounds crazy but maybe it is worth considering beta backporting this? Only if it's not a lot of work, of course! But yeah, stabilized array::map is already in beta and if this problem is really as big as it sometimes seems, might be worth having the docs in place when 1.55 is released. CC `@CryZe` r? `@m-ou-se` (since you were involved in that discussion and the stabilization)
This was referenced Jul 30, 2021
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 30, 2021
Rollup of 9 pull requests Successful merges: - rust-lang#86072 (Cross compiling rustc_llvm on Darwin requires zlib.) - rust-lang#87385 (Make `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` warn by default) - rust-lang#87547 (Add missing examples for NonNull) - rust-lang#87557 (Fix issue with autofix for ambiguous associated function from Rust 2021 prelude when struct is generic) - rust-lang#87559 (Tweak borrowing suggestion in `for` loop) - rust-lang#87596 (Add warning when whitespace is not skipped after an escaped newline) - rust-lang#87606 (Add some TAIT-related regression tests) - rust-lang#87609 (Add docs about performance and `Iterator::map` to `[T; N]::map`) - rust-lang#87616 (Fix missing word in rustdoc book) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This suboptimal code gen for some usages of array::map got a bit of
attention by multiple people throughout the community. Some cases:
array_map#75243 (comment)array_map#75243 (comment)My guess is that this gets the attention it gets because in JavaScript
(and potentially other languages), a
mapfunction on arrays is verycommonly used since in those languages, arrays basically take the role
of Rust's iterator. I considered explicitly naming JavaScript in the
first paragraph I added, but I couldn't find precedence of mentioning
other languages in standard library doc, so I didn't add it.
When array::map was stabilized, we still wanted to add docs, but that
somehow did not happen in time. So here we are. Not sure if this sounds
crazy but maybe it is worth considering beta backporting this? Only if
it's not a lot of work, of course! But yeah, stabilized array::map is
already in beta and if this problem is really as big as it sometimes seems,
might be worth having the docs in place when 1.55 is released.
CC @CryZe
r? @m-ou-se (since you were involved in that discussion and the stabilization)