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

Tracking issue for RFC 2521, "Unify std::os::raw::c_void and libc::c_void via libcore" #53856

Closed
1 of 3 tasks
Centril opened this issue Aug 31, 2018 · 10 comments
Closed
1 of 3 tasks

Tracking issue for RFC 2521, "Unify std::os::raw::c_void and libc::c_void via libcore" #53856

Centril opened this issue Aug 31, 2018 · 10 comments
Assignees
@SimonSapin
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Aug 31, 2018 •
edited by SimonSapin
Loading

This is a tracking issue for the RFC "Unify std::os::raw::c_void and libc::c_void via libcore" (rust-lang/rfcs#2521).

Steps:

Unresolved questions:

There are none.
@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Aug 31, 2018
@Centril Centril mentioned this issue Aug 31, 2018
Merged
@Ericson2314
Copy link
Contributor

OK per what @retep998 was saying, if we can get Cargo to ensure that use_std is always used when std is used, we can and should avoid putting anything in libcore.

@alexcrichton
Copy link
Member

@Ericson2314 without a design of how to do that it's simply not an option. There is no ability to do that today, and it's not clear how that would be done (nor whether it's desirable).

@IsaacWoods
Copy link
Contributor

I'd like to work on this :)

@IsaacWoods IsaacWoods mentioned this issue Sep 2, 2018
Merged
bors added a commit that referenced this issue Sep 16, 2018
@bors
Auto merge of #53910 - IsaacWoods:unify_cvoid, r=SimonSapin
5aac93c 
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
@IsaacWoods IsaacWoods mentioned this issue Sep 17, 2018
Merged
bors added a commit to rust-lang/libc that referenced this issue Sep 19, 2018
@bors
Auto merge of #1082 - IsaacWoods:core_cvoid, r=alexcrichton
d13f346 
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
@IsaacWoods
Copy link
Contributor

Afaik the implementation for this is now complete and will be insta-stable from 1.31.0

@SimonSapin
Copy link
Contributor

Thanks for your PRs @IsaacWoods!

I believe core::ffi::c_void is also available in 1.30.0. What do you think of having libc’s build script do feature detection (run rustc on a dummy file that imports core::ffi::c_void) instead of version detection like rust-lang/libc#1082 does?

@IsaacWoods
Copy link
Contributor

I went with 1.31.0 to make sure not to break nightly users using a 1.30.0 libstd without the change but upgrading libc, but feature detection would probably be better. I’ll send another PR when I have the chance

@IsaacWoods IsaacWoods mentioned this issue Oct 1, 2018
Merged
bors added a commit to rust-lang/libc that referenced this issue Oct 4, 2018
@bors
Auto merge of #1086 - IsaacWoods:master, r=alexcrichton
bd42d06 
Revisit work on cvoid

At the recommendation of @SimonSapin [here](rust-lang/rust#53856 (comment)), I have revisited the build script to check whether `core::ffi::c_void` resolves, instead of relying on a particular `rustc` version. This allows use on `1.30.x` builds with `core::ffi::c_void`.

I also noticed that `c_void` is defined twice in this crate, once in `lib.rs` and again in `switch.rs`. This instead defines `c_void` for every target except `wasm32`. As far as I can tell, this shouldn't actually change functionality on any existing targets.
bors added a commit to rust-lang/libc that referenced this issue Oct 5, 2018
@bors
Auto merge of #1086 - IsaacWoods:master, r=alexcrichton
57c4a15 
Revisit work on cvoid

At the recommendation of @SimonSapin [here](rust-lang/rust#53856 (comment)), I have revisited the build script to check whether `core::ffi::c_void` resolves, instead of relying on a particular `rustc` version. This allows use on `1.30.x` builds with `core::ffi::c_void`.

I also noticed that `c_void` is defined twice in this crate, once in `lib.rs` and again in `switch.rs`. This instead defines `c_void` for every target except `wasm32`. As far as I can tell, this shouldn't actually change functionality on any existing targets.
@kHigasa
Copy link

kHigasa commented Nov 26, 2018

I could confirm that this issue was fixed. I was able to build correctly with rustc 1.32.0-nightly (6acbb5b65 2018-11-25) :)

Closed
@elichai
Copy link
Contributor

elichai commented Feb 18, 2019 •
edited
Loading

why not move all std::os::raw to libcore too?

Edit:
Found the RFC about it, sorry. rust-lang/rfcs#1783

@SimonSapin
Copy link
Contributor

https://github.com/rust-lang/rfcs/blob/master/text/2521-c_void-reunification.md

This has been extensively discussed already:

In some of those discussions, there was no consensus for having integer type aliases in libcore because some of them are platform-specific but libcore “should be pure”.

@SimonSapin
Copy link
Contributor

This was insta-stable (with FCP) in #53910, so closing as it seems there is nothing else to track here. Feel free to reopen if there is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SimonSapin SimonSapin

Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@alexcrichton @SimonSapin @Centril @Ericson2314 @elichai @IsaacWoods @kHigasa