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

Docs: slice elements are equidistant #64703

Closed
wants to merge 1 commit into from

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Sep 23, 2019

Recently, someone asked why [char] and str are not interchangeable, and I explained that in a slice, the elements must be laid out equidistantly, whereas the chars in a str are stored compactly regardless their size. However I couldn't find this documented anywhere, so here's a small addition of this fact.

@rust-highfive
Copy link
Collaborator

r? @rkruppe

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 23, 2019
@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 23, 2019

the elements must be laid out equidistantly

I understood this from the word "contiguous". Maybe instead of introducing a new word ("equidistantly"), it might be more helpful to explain in the body what contiguous means, e.g.,

/// "Contiguous" means that there are no gaps between the elements, that is, 
/// the `i`-th slice field is located at offset `i * size_of::<T>()`.  

but maybe trying to word that in a beginner-friendly way.

@sinkuu
Copy link
Contributor

sinkuu commented Sep 23, 2019

"constant stride" is the word used in the Unsafe Code Guidelines. (Also, it's possible that stride == size does not hold in the future Rust versions, according to this page.)

@hanna-kruppe
Copy link
Contributor

I am not a fan of the UCG trying to accommodate hypothetical future "stride != size" changes (see discussion in rust-lang/unsafe-code-guidelines#176) so I'm not thrilled to see it make its way into some[1] libstd docs. Can we make the intended clarification without opening this can of worms?

Perhaps something like @gnzlbg propoed earlier, but avoid mentioning size_of directly? Perhaps:

/// "Contiguous" means that there are no gaps between the elements, so
/// every element is the same distance from its neighbors.
[1] Some... but not all! This is one of my complaints about it in the the UCG too, that the carefully couched language is not applied consistently enough to actually make a dent in future compatibility.

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 27, 2019 via email

@llogiq
Copy link
Contributor Author

llogiq commented Sep 27, 2019

Ok, I removed the "stride" part as per @rkruppe's request. r?

@hanna-kruppe
Copy link
Contributor

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 27, 2019

📌 Commit 6ccb7ae has been approved by rkruppe

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2019
@llogiq
Copy link
Contributor Author

llogiq commented Sep 27, 2019

Thanks @rkruppe!

@@ -566,7 +566,9 @@ mod prim_array { }
#[doc(alias = "[")]
#[doc(alias = "]")]
#[doc(alias = "[]")]
/// A dynamically-sized view into a contiguous sequence, `[T]`.
/// A dynamically-sized view into a contiguous sequence, `[T]`. Contiguous here
/// means that elements are layed out so that every element is the same
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/layed out/laid out/

Had to look this one up myself: https://writingexplained.org/laid-out-or-layed-out

Centril added a commit to Centril/rust that referenced this pull request Sep 28, 2019
…, r=rkruppe

Docs: slice elements are equidistant

Recently, someone asked why `[char]` and `str` are not interchangeable, and I explained that in a slice, the elements must be laid out equidistantly, whereas the chars in a `str` are stored compactly regardless their size. However I couldn't find this documented anywhere, so here's a small addition of this fact.
bors added a commit that referenced this pull request Sep 28, 2019
Rollup of 14 pull requests

Successful merges:

 - #63492 (Remove redundancy from the implementation of C variadics.)
 - #64703 (Docs: slice elements are equidistant)
 - #64745 (Include message on tests that should panic but do not)
 - #64781 (Remove stray references to the old global tcx)
 - #64794 (Remove unused DepTrackingMap)
 - #64802 (Account for tail expressions when pointing at return type)
 - #64809 (hir: Disallow `target_feature` on constants)
 - #64815 (Fix div_duration() marked as stable by mistake)
 - #64818 (update rtpSpawn's parameters type(It's prototype has been updated in libc))
 - #64830 (Thou shallt not `.abort_if_errors()`)
 - #64836 (Stabilize map_get_key_value feature)
 - #64845 (pin.rs: fix links to primitives in documentation)
 - #64847 (Upgrade env_logger to 0.7)
 - #64851 (Add mailmap entry for Dustin Bensing by request)

Failed merges:

 - #64824 (No StableHasherResult everywhere)

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 28, 2019
…, r=rkruppe

Docs: slice elements are equidistant

Recently, someone asked why `[char]` and `str` are not interchangeable, and I explained that in a slice, the elements must be laid out equidistantly, whereas the chars in a `str` are stored compactly regardless their size. However I couldn't find this documented anywhere, so here's a small addition of this fact.
bors added a commit that referenced this pull request Sep 28, 2019
Rollup of 14 pull requests

Successful merges:

 - #64703 (Docs: slice elements are equidistant)
 - #64745 (Include message on tests that should panic but do not)
 - #64781 (Remove stray references to the old global tcx)
 - #64794 (Remove unused DepTrackingMap)
 - #64802 (Account for tail expressions when pointing at return type)
 - #64809 (hir: Disallow `target_feature` on constants)
 - #64815 (Fix div_duration() marked as stable by mistake)
 - #64818 (update rtpSpawn's parameters type(It's prototype has been updated in libc))
 - #64830 (Thou shallt not `.abort_if_errors()`)
 - #64836 (Stabilize map_get_key_value feature)
 - #64845 (pin.rs: fix links to primitives in documentation)
 - #64847 (Upgrade env_logger to 0.7)
 - #64851 (Add mailmap entry for Dustin Bensing by request)
 - #64859 (check_match: improve diagnostics for `let A = 2;` with `const A: i32 = 3`)

Failed merges:

r? @ghost
@llogiq
Copy link
Contributor Author

llogiq commented Sep 28, 2019

Thanks @petertodd for the correction! @rkruppe can you reapprove? Sorry @Centril if this interferes with your roll-up.

@Centril
Copy link
Contributor

Centril commented Sep 28, 2019

(Noted also on Discord):

Force pushing to a PR in a rollup has no effect on the rollup (which should not be cancelled unless there's a serious problem in the PR (a typo does not qualify)). You will need to rebase atop upstream-master after the rollup has been merged.

@bors
Copy link
Contributor

bors commented Sep 28, 2019

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

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 28, 2019
@llogiq llogiq mentioned this pull request Sep 28, 2019
Centril added a commit to Centril/rust that referenced this pull request Sep 28, 2019
Slice docs: fix typo

With rust-lang#64703, I introduced a typo. Here is the fix. Sorry for the inconvenience.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants