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.

Sign up for GitHub

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

Jump to bottom

Improve some pointer-related documentation #62822

Merged
merged 8 commits into from
Jul 26, 2019
Merged

Improve some pointer-related documentation #62822

merged 8 commits into from
Jul 26, 2019

Conversation

RalfJung
Copy link
Member

No description provided.

@rust-highfive
Copy link
Collaborator

r? @sfackler

(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 20, 2019
Centril
Centril reviewed Jul 20, 2019
View reviewed changes
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
src/libstd/primitive_docs.rs Show resolved Hide resolved
src/libstd/primitive_docs.rs Show resolved Hide resolved
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Jul 20, 2019

r? @Centril

@rust-highfive rust-highfive assigned Centril and unassigned sfackler Jul 20, 2019
@Centril
Copy link
Contributor

Centril commented Jul 22, 2019

Thanks! r=me rollup with #62822 (comment) addressed in some way.

@Centril
Copy link
Contributor

Centril commented Jul 22, 2019

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 22, 2019

📌 Commit 9196781 has been approved by Centril

@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 22, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 22, 2019
@Centril
Rollup merge of rust-lang#62822 - RalfJung:pointers, r=Centril
1d5ea88 
Improve some pointer-related documentation
@Centril Centril mentioned this pull request Jul 22, 2019
Closed
gnzlbg
gnzlbg reviewed Jul 23, 2019
View reviewed changes
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
src/libstd/primitive_docs.rs Outdated Show resolved Hide resolved
src/libstd/primitive_docs.rs Show resolved Hide resolved
@RalfJung
Copy link
Member Author

@bors r-

will look at the comments later

@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 23, 2019
@RalfJung @gnzlbg
Apply suggestions from code review
dd5045e 
Co-Authored-By: gnzlbg <gnzlbg@users.noreply.github.com>
@RalfJung
Copy link
Member Author

@gnzlbg

This compiles on stable Rust, so if the Rust language isn't ok with it then somebody should open a soundness bug (playground).

Somebody did: #28179.

@RalfJung
Copy link
Member Author

@Centril please have a look at the changes; the one about references is a new sentence so I didn't feel comfortable just doing r=Centril.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 23, 2019 •
edited
Loading

@RalfJung there is no symbol defined twice in those playground examples (note the #![no_main] attribute), also the issue here is Rust accepting the static mut - the version without the mut is ok, as niko shows in that thread.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 23, 2019 •
edited
Loading

That issue is about no_mangle being unsafe in general; defining a symbol twice is just one of many ways.

Also see #28179 (comment) (EDIT: I see you edited your post to mention that). And no that version is not okay, it runs arbitrary unsafe code without unsafe.

@Centril
Copy link
Contributor

Centril commented Jul 23, 2019

@Centril please have a look at the changes; the one about references is a new sentence so I didn't feel comfortable just doing r=Centril.

Thanks; still looks good :)

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 23, 2019

📌 Commit 65cf10d has been approved by Centril

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 23, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
@Centril
Rollup merge of rust-lang#62822 - RalfJung:pointers, r=Centril
b71e579 
Improve some pointer-related documentation
@Centril Centril mentioned this pull request Jul 24, 2019
Closed
bors added a commit that referenced this pull request Jul 24, 2019
@bors
Auto merge of #62923 - Centril:rollup-53i3am3, r=Centril
486833d 
Rollup of 11 pull requests

Successful merges:

 - #62261 (Take substs into account in `conservative_is_privately_uninhabited`)
 - #62528 (Add joining slices of slices with a slice separator, not just a single item)
 - #62738 (Remove uses of mem::uninitialized from std::sys::cloudabi)
 - #62784 (Add riscv32i-unknown-none-elf target)
 - #62808 (Revert "Disable stack probing for gnux32.")
 - #62814 (add support for hexagon-unknown-linux-musl)
 - #62822 (Improve some pointer-related documentation)
 - #62890 (Normalize use of backticks in compiler messages for libsyntax/*)
 - #62901 (cleanup: Remove `extern crate serialize as rustc_serialize`s)
 - #62905 (Normalize use of backticks in compiler messages for doc)
 - #62908 (normalize use of backticks for compiler messages in remaining modules)

Failed merges:

r? @ghost
@RalfJung RalfJung mentioned this pull request Jul 24, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
@Centril
Rollup merge of rust-lang#62822 - RalfJung:pointers, r=Centril
a6a0122 
Improve some pointer-related documentation
@Centril Centril mentioned this pull request Jul 24, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 24, 2019
@Centril
Rollup merge of rust-lang#62960 - Centril:rollup-8jay7g7, r=Centril
ab09144 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#60938 (rustdoc: make #[doc(include)] relative to the containing file)
 - rust-lang#61890 (Fix some sanity checks)
 - rust-lang#62084 (allow clippy::unreadable_literal in unicode tables)
 - rust-lang#62261 (Take substs into account in `conservative_is_privately_uninhabited`)
 - rust-lang#62528 (Add joining slices of slices with a slice separator, not just a single item)
 - rust-lang#62735 (Turn `#[global_allocator]` into a regular attribute macro)
 - rust-lang#62801 (Remove support for -Zlower-128bit-ops)
 - rust-lang#62808 (Revert "Disable stack probing for gnux32.")
 - rust-lang#62822 (Improve some pointer-related documentation)
 - rust-lang#62904 (Disable d32 on armv6 hf targets)
 - rust-lang#62907 (Initialize the MSP430 AsmParser)
 - rust-lang#62921 (Add method disambiguation help for trait implementation)
 - rust-lang#62942 (Use match ergonomics in Condvar documentation)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Jul 25, 2019
@Centril
Rollup merge of rust-lang#62822 - RalfJung:pointers, r=Centril
8bc8562 
Improve some pointer-related documentation
@Centril Centril mentioned this pull request Jul 25, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jul 25, 2019
@Centril
Rollup merge of rust-lang#62822 - RalfJung:pointers, r=Centril
a534c37 
Improve some pointer-related documentation
@Centril Centril mentioned this pull request Jul 25, 2019
Merged
bors added a commit that referenced this pull request Jul 25, 2019
@bors
Auto merge of #62990 - Centril:rollup-k9n0hvs, r=Centril
1863067 
Rollup of 15 pull requests

Successful merges:

 - #60066 (Stabilize the type_name intrinsic in core::any)
 - #60938 (rustdoc: make #[doc(include)] relative to the containing file)
 - #61884 (Stablize Euclidean Modulo (feature euclidean_division))
 - #61890 (Fix some sanity checks)
 - #62528 (Add joining slices of slices with a slice separator, not just a single item)
 - #62707 (Add tests for overlapping explicitly dropped locals in generators)
 - #62735 (Turn `#[global_allocator]` into a regular attribute macro)
 - #62822 (Improve some pointer-related documentation)
 - #62887 (Make the parser TokenStream more resilient after mismatched delimiter recovery)
 - #62921 (Add method disambiguation help for trait implementation)
 - #62930 (Add test for #51559)
 - #62942 (Use match ergonomics in Condvar documentation)
 - #62977 (Fix inconsistent highlight blocks.)
 - #62978 (Remove `cfg(bootstrap)` code for array implementations)
 - #62981 (Add note suggesting to borrow a String argument to find)

Failed merges:

 - #62964 (clarify and unify some type test names)

r? @ghost
@bors bors merged commit 65cf10d into rust-lang:master Jul 26, 2019
@RalfJung RalfJung deleted the pointers branch July 29, 2019 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@gnzlbg gnzlbg gnzlbg left review comments

@Lokathor Lokathor Lokathor left review comments

Assignees

@Centril Centril

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@RalfJung @rust-highfive @Centril @bors @gnzlbg @Lokathor @sfackler