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

#62357: doc(ptr): add example for {read,write}_unaligned #62493

Merged
merged 1 commit into from
Jul 11, 2019

Conversation

Freyskeyd
Copy link
Contributor

@Freyskeyd Freyskeyd commented Jul 8, 2019

related to #62357

With #62323 the only example (that had UB and was thus invalid) in std::ptr::read_unaligned and std::ptr::write_unaligned is removed.

We should add a valid example of using the aforementioned functions.

Signed-off-by: Freyskeyd simon.paitrault@gmail.com

@rust-highfive
Copy link
Collaborator

r? @Kimundi

(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 Jul 8, 2019
@Centril
Copy link
Contributor

Centril commented Jul 8, 2019

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned Kimundi Jul 8, 2019
@RalfJung
Copy link
Member

RalfJung commented Jul 8, 2019

Thanks a lot!
I'm on a deadline crunch right now; I'll start working on my backlog by the end of the week.

Also, this PR is marked as not-yet-ready-for-review?

@Freyskeyd
Copy link
Contributor Author

@ralfbiedert Yes I marked it has draft because it's one of my first PR, didn't know if I needed to do it before switching to "ready for review".

@RalfJung
Copy link
Member

RalfJung commented Jul 8, 2019

Well, if you want it to be reviewed, then mark it as ready for that. :)

@Freyskeyd Freyskeyd marked this pull request as ready for review July 8, 2019 14:57
@ralfbiedert
Copy link
Contributor

@Freyskeyd I guess you mean @RalfJung :)

Copy link
Contributor

@hanna-kruppe hanna-kruppe left a comment

Choose a reason for hiding this comment

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

Since @RalfJung is busy, r? @rkruppe

The idea of these examples look good to me, thanks! There's some nits that need to be fixed, though.

src/libcore/ptr/mod.rs Outdated Show resolved Hide resolved
src/libcore/ptr/mod.rs Outdated Show resolved Hide resolved
src/libcore/ptr/mod.rs Outdated Show resolved Hide resolved
src/libcore/ptr/mod.rs Show resolved Hide resolved
@hanna-kruppe hanna-kruppe assigned hanna-kruppe and unassigned RalfJung Jul 8, 2019
@Freyskeyd Freyskeyd force-pushed the valid_example_read-write_unaligned branch from 6750518 to 185dda3 Compare July 8, 2019 15:19
src/libcore/ptr/mod.rs Outdated Show resolved Hide resolved
@Freyskeyd Freyskeyd force-pushed the valid_example_read-write_unaligned branch from 185dda3 to 7ef7e93 Compare July 9, 2019 07:11
@Freyskeyd
Copy link
Contributor Author

@lzutao @rkruppe I fixed the examples :)

@hanna-kruppe
Copy link
Contributor

Thanks! CI is failing, though. From glancing at the logs it seems the problem is just that mem::size_of doesn't resolve. Adding use std::mem; should fix it?

Signed-off-by: Freyskeyd <simon.paitrault@gmail.com>
@Freyskeyd Freyskeyd force-pushed the valid_example_read-write_unaligned branch from 7ef7e93 to bc322af Compare July 9, 2019 14:55
@Freyskeyd
Copy link
Contributor Author

@rkruppe is it ok for you ? :)

@hanna-kruppe
Copy link
Contributor

CI passes, that's good :) Thanks again!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 10, 2019

📌 Commit bc322af 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 Jul 10, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 10, 2019
…unaligned, r=rkruppe

rust-lang#62357: doc(ptr): add example for {read,write}_unaligned

related to rust-lang#62357

> With rust-lang#62323 the only example (that had UB and was thus invalid) in std::ptr::read_unaligned and std::ptr::write_unaligned is removed.

> We should add a valid example of using the aforementioned functions.

Signed-off-by: Freyskeyd <simon.paitrault@gmail.com>
bors added a commit that referenced this pull request Jul 10, 2019
Rollup of 5 pull requests

Successful merges:

 - #62275 (rustc_mir: treat DropAndReplace as Drop + Assign in qualify_consts.)
 - #62465 (Sometimes generate storage statements for temporaries with type `!`)
 - #62481 (Use `fold` in `Iterator::last` default implementation)
 - #62493 (#62357: doc(ptr): add example for {read,write}_unaligned)
 - #62532 (Some more cleanups to syntax::print)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Jul 10, 2019
Rollup of 5 pull requests

Successful merges:

 - #62275 (rustc_mir: treat DropAndReplace as Drop + Assign in qualify_consts.)
 - #62465 (Sometimes generate storage statements for temporaries with type `!`)
 - #62481 (Use `fold` in `Iterator::last` default implementation)
 - #62493 (#62357: doc(ptr): add example for {read,write}_unaligned)
 - #62532 (Some more cleanups to syntax::print)

Failed merges:

r? @ghost
@bors bors merged commit bc322af into rust-lang:master Jul 11, 2019
@bors
Copy link
Contributor

bors commented Jul 11, 2019

☔ The latest upstream changes (presumably #62561) 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 Jul 11, 2019
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.

9 participants