-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Move std::os::raw::c_void into libcore and re-export in libstd #53910
Conversation
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Looks good, thanks! As Travis-CI suggests, this needs @rust-lang/libs, how do you feel about @rfcbot merge |
Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Rather than changing the links, maybe it’d be better to add |
@SimonSapin Tried this and tested locally (
I don't think we could use the same link from all three places ( |
Sorry, in my previous comment I was only thinking of links to Indeed, So we probably need to not use |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Fair enough, I was hoping it would be to the root of the documentation, so e.g. With no |
@rust-lang/docs, any thoughts on how we should handle this? |
Update: adding The exact error from linkchecker is:
|
Ping @rust-lang/docs & @rust-lang/rustdoc: It looks like there's some issues with |
Links in the std docs are incredibly fragile. Since this is something that exists in both core and std, you need to make sure that they're at the same directory level in both places. In fact, looking at the current Travis error, you may be closer then you think...
It seems like it's only complaining about the re-export in What's happening here is that the link you gave it is relative to its file path. This enum appears in three places in this PR:
Note that the module path for that last one is longer than the other two. This means that it has one more folder parent to pass through with (Strictly speaking, this is what "intra-doc links" were made to fix, but i'm not sure the primitive link will totally work out? For one, you'd need to add the reference into this array, but also, the primitive docs aren't even available in libcore! We'd have to fix rustdoc to make sure the link is handled correctly.) |
@QuietMisdreavus if we make
|
|
@SimonSapin @QuietMisdreavus thank you for your help, linkchecker is now passing. Does anything else need to be done for this PR, maybe a squash? |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
r? @SimonSapin |
Oops I didn’t realize the squash was done. Looks good, thanks! @bors r+ |
📌 Commit 23e345b has been approved by |
Move std::os::raw::c_void into libcore and re-export in libstd Implements the first part of [RFC 2521](rust-lang/rfcs#2521). cc #53856
☀️ Test successful - status-appveyor, status-travis |
📣 Toolstate changed by #53910! Tested on commit 5aac93c. 💔 clippy-driver on windows: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). |
Tested on commit rust-lang/rust@5aac93c. Direct link to PR: <rust-lang/rust#53910> 💔 clippy-driver on windows: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → test-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
This got changed in rust-lang/rust#53910
Update Clippy Fix Clippy test-fail due to #53910 r? @Manishearth @SimonSapin
Re-export core::ffi::c_void if it exists This is the second part of the implementation of [RFC 2521](rust-lang/rfcs#2521), replacing the definition of `c_void` in libc with a re-export of the type from `core::ffi::c_void` on builds it exists for. This uses the re-export for rustc version `1.31.0` or greater, as `1.30.x` was the current nightly when [the PR for the changes to libcore and libstd](rust-lang/rust#53910) was merged, so I'm assuming the first nightly they will appear in will be `1.31.0`; is this acceptable? cc rust-lang/rust#53856
Implements the first part of RFC 2521.
cc #53856