clarify pointer add/sub function safety concerns #55060
Merged
Conversation
This file contains hidden or 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 has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
joshtriplett
requested changes
Oct 14, 2018
src/libcore/ptr.rs
Outdated
| @@ -1255,7 +1255,7 @@ impl<T: ?Sized> *const T { | |||
| /// Behavior: | |||
| /// | |||
| /// * Both the starting and resulting pointer must be either in bounds or one | |||
| /// byte past the end of an allocated object. | |||
| /// byte past the end of *the same* allocated object. | |||
There was a problem hiding this comment.
The emphasis here feels unnecessary; it only makes sense in the context of a patch, not in the context of the resulting changed text.
There was a problem hiding this comment.
The emphasis was copied from identical sentences on the offset functions. Shall I remove it there, as well?
There was a problem hiding this comment.
Removed the emphases in doc comments on offset() in a separate commit.
There was a problem hiding this comment.
Ah, I missed that!
It's not that important, but thanks for updating those for consistency as well,
src/libcore/ptr.rs
Outdated
| @@ -1312,7 +1312,7 @@ impl<T: ?Sized> *const T { | |||
| /// Behavior: | |||
| /// | |||
| /// * Both the starting and resulting pointer must be either in bounds or one | |||
| /// byte past the end of an allocated object. | |||
| /// byte past the end of *the same* allocated object. | |||
src/libcore/ptr.rs
Outdated
| @@ -1893,7 +1893,7 @@ impl<T: ?Sized> *mut T { | |||
| /// Behavior: | |||
| /// | |||
| /// * Both the starting and resulting pointer must be either in bounds or one | |||
| /// byte past the end of an allocated object. | |||
| /// byte past the end of *the same* allocated object. | |||
src/libcore/ptr.rs
Outdated
| @@ -1950,7 +1950,7 @@ impl<T: ?Sized> *mut T { | |||
| /// Behavior: | |||
| /// | |||
| /// * Both the starting and resulting pointer must be either in bounds or one | |||
| /// byte past the end of an allocated object. | |||
| /// byte past the end of *the same* allocated object. | |||
Ralf Jung made the same changes to the offset functions' documentation in commit fb08915. As add/sub just call offset, the same limitation applies here, as well. Removed emphasis on review request by @joshtriplett
During review of the previous commit, @joshtriplett noticed that the emphasis on 'the same' is unnecessary. For consistency, remove it on the offset() functions, as well.
|
@bors r+ rollup |
|
📌 Commit 6cc84ac has been approved by |
bors
added
S-waiting-on-bors
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Oct 15, 2018
clarify pointer add/sub function safety concerns Ralf Jung made the same changes to the offset functions' documentation in commit fb08915. As add/sub just call offset, the same limitation applies here, as well. I did not copy the whole explanation ("In particular, the resulting pointer may *not* be used to access a different allocated object [...]") because I'd consider that as being too repetitive. The documentation of add/sub already refers to the offset function, so people interested in the details can look it up, there. But changing 'an object' to 'the same object' is a small change which improves clarity a lot.
bors
added a commit
that referenced
this pull request
Oct 15, 2018
Rollup of 11 pull requests Successful merges: - #54820 (Closes #54538: `unused_patterns` lint) - #54963 (Cleanup rustc/session) - #54991 (add test for #23189) - #55025 (Add missing lifetime fragment specifier to error message.) - #55047 (doc: make core::fmt::Error example more simple) - #55048 (Don't collect to vectors where unnecessary) - #55060 (clarify pointer add/sub function safety concerns) - #55062 (Make EvalContext::step public again) - #55066 (Fix incorrect link in println! documentation) - #55081 (Deduplicate tests) - #55088 (Update rustc documentation link) Failed merges: r? @ghost
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.
Ralf Jung made the same changes to the offset functions' documentation
in commit fb08915. As add/sub just call offset, the same limitation
applies here, as well.
I did not copy the whole explanation ("In particular, the resulting pointer may not be used to access a different allocated object [...]") because I'd consider that as being too repetitive. The documentation of add/sub already refers to the offset function, so people interested in the details can look it up, there.
But changing 'an object' to 'the same object' is a small change which improves clarity a lot.