Skip to content

Move std::sys_common::alloc to new module std::sys::common #82492

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

CDirkx
Copy link
Contributor

@CDirkx CDirkx commented Feb 24, 2021

//! The relationship between `std::sys_common`, `std::sys` and the
//! rest of `std` is complex, with dependencies going in all
//! directions: `std` depending on `sys_common`, `sys_common`
//! depending on `sys`, and `sys` depending on `sys_common` and `std`.
//! Ideally `sys_common` would be split into two and the dependencies
//! between them all would form a dag, facilitating the extraction of
//! `std::sys` from the standard library.

It was my impression that the goal for std::sys has changed from extracting it into a separate crate to making std work with features. However the fact remains that there is a lot of interdependence between sys and sys_common, this is because sys_common contains two types of code:

This PR attempts to address this by adding a new module common to std::sys which will contain code shared between platforms, alloc.rs in this case but more can be moved over in the future.

@rust-highfive
Copy link
Contributor

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 Feb 24, 2021
@crlf0710 crlf0710 added T-libs Relevant to the library team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 12, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2021
@Dylan-DPC-zz
Copy link

r? @m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned sfackler Mar 29, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Apr 14, 2021

This looks good to me, but I'm a bit worried about the existence of both sys::common and sys_common being confusing. Could you add a comment to sys/common.rs and sys_common/mod.rs explaining the difference?

Thanks!

@m-ou-se m-ou-se 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2021
@CDirkx
Copy link
Contributor Author

CDirkx commented Apr 14, 2021

I agree on the confusion. Once all the common code is eventually split out of sys_common it might make sense to rename it to something like sys_interface or something, as at that point it would mostly only be a facade for sys. But that is for much later, I'll update the documentation in sys_common and add some in sys::common.

@CDirkx CDirkx force-pushed the sys_common_alloc branch from 79f22fd to cac0dd6 Compare April 14, 2021 12:03
@CDirkx
Copy link
Contributor Author

CDirkx commented Apr 14, 2021

I added some documentation and set up #84187 to track the progress of moving over code from sys_common to sys::common in future PRs.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 14, 2021

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 14, 2021

📌 Commit cac0dd6 has been approved by m-ou-se

@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 Apr 14, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2021
Move `std::sys_common::alloc` to new module `std::sys::common`

https://github.com/rust-lang/rust/blob/6b56603e35b39c9f6cc76782330e5e415f9e43d5/library/std/src/sys_common/mod.rs#L7-L13

It was my impression that the goal for `std::sys` has changed from extracting it into a separate crate to making std work with features. However the fact remains that there is a lot of interdependence between `sys` and `sys_common`, this is because `sys_common` contains two types of code:

- abstractions over the different platform implementations in `std::sys` (for example [`std::sys_common::mutex`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys_common/mutex.rs))
- code shared between platforms (for example [`std::sys_common::alloc`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys_common/alloc.rs))

This PR attempts to address this by adding a new module `common` to `std::sys` which will contain code shared between platforms, `alloc.rs` in this case but more can be moved over in the future.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2021
Move `std::sys_common::alloc` to new module `std::sys::common`

https://github.com/rust-lang/rust/blob/6b56603e35b39c9f6cc76782330e5e415f9e43d5/library/std/src/sys_common/mod.rs#L7-L13

It was my impression that the goal for `std::sys` has changed from extracting it into a separate crate to making std work with features. However the fact remains that there is a lot of interdependence between `sys` and `sys_common`, this is because `sys_common` contains two types of code:

- abstractions over the different platform implementations in `std::sys` (for example [`std::sys_common::mutex`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys_common/mutex.rs))
- code shared between platforms (for example [`std::sys_common::alloc`](https://github.com/rust-lang/rust/blob/master/library/std/src/sys_common/alloc.rs))

This PR attempts to address this by adding a new module `common` to `std::sys` which will contain code shared between platforms, `alloc.rs` in this case but more can be moved over in the future.
This was referenced Apr 14, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#82492 (Move `std::sys_common::alloc` to new module `std::sys::common`)
 - rust-lang#84177 (Fix join_paths error display.)
 - rust-lang#84185 (add more pat2021 tests)
 - rust-lang#84191 (Update books)
 - rust-lang#84192 (Fix typos in rustc_codegen_ssa/src/back/write.rs.)
 - rust-lang#84196 (:arrow_up: rust-analyzer)
 - rust-lang#84201 (rustdoc: Note that forbidding anchors in links to primitives is a bug)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 80ee7cb into rust-lang:master Apr 15, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 15, 2021
@bors
Copy link
Collaborator

bors commented Apr 15, 2021

⌛ Testing commit cac0dd6 with merge 9c3b66c...

@CDirkx CDirkx deleted the sys_common_alloc branch April 15, 2021 05:35
@CDirkx CDirkx mentioned this pull request Apr 27, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants