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

Restrict where in the tree platform-specific cfgs may be mentioned #36807

Merged
merged 7 commits into from Oct 3, 2016

Conversation

Projects
None yet
5 participants
@brson
Contributor

brson commented Sep 28, 2016

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where cfg(unix),
cfg(windows), cfg(target_os) and cfg(target_env) may appear,
the basic objective being to isolate platform-specific code to the
platform-specific std::sys modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

  • core may not have platform-specific code
  • liballoc_system may have platform-specific code
  • liballoc_jemalloc may have platform-specific code
  • libpanic_abort may have platform-specific code
  • libpanic_unwind may have platform-specific code
  • other crates in the std facade may not
  • std may have platform-specific code in the following places
    • sys/unix/
    • sys/windows/
    • os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only std::sys (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Sep 28, 2016

Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

Collaborator

rust-highfive commented Sep 28, 2016

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Sep 29, 2016

Member

Looks great to me!

Member

alexcrichton commented Sep 29, 2016

Looks great to me!

src/libstd/sys/windows/env.rs
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[cfg(target_os = "windows")]

This comment has been minimized.

@alexcrichton

alexcrichton Sep 29, 2016

Member

This can be removed I think

@alexcrichton

alexcrichton Sep 29, 2016

Member

This can be removed I think

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 29, 2016

Contributor

Updated.

Contributor

brson commented Sep 29, 2016

Updated.

@brson

This comment has been minimized.

Show comment
Hide comment
Contributor

brson commented Sep 29, 2016

@rust-highfive rust-highfive assigned alexcrichton and unassigned aturon Sep 29, 2016

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 29, 2016

Contributor

I did some more refactoring, moving the argument handling around. That'll surely bounce a few times on misc platforms...

Contributor

brson commented Sep 29, 2016

I did some more refactoring, moving the argument handling around. That'll surely bounce a few times on misc platforms...

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Sep 29, 2016

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 29, 2016

Contributor

📌 Commit 2086608 has been approved by alexcrichton

Contributor

bors commented Sep 29, 2016

📌 Commit 2086608 has been approved by alexcrichton

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 29, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Sep 29, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 29, 2016

Contributor

📌 Commit 54440be has been approved by alexcrichton

Contributor

bors commented Sep 29, 2016

📌 Commit 54440be has been approved by alexcrichton

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 30, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Sep 30, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit 5cf31ba has been approved by alexcrichton

Contributor

bors commented Sep 30, 2016

📌 Commit 5cf31ba has been approved by alexcrichton

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 30, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Sep 30, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit 1472c45 has been approved by alexcrichton

Contributor

bors commented Sep 30, 2016

📌 Commit 1472c45 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

⌛️ Testing commit 1472c45 with merge dbcc5b1...

Contributor

bors commented Sep 30, 2016

⌛️ Testing commit 1472c45 with merge dbcc5b1...

bors added a commit that referenced this pull request Sep 30, 2016

Auto merge of #36807 - brson:pal, r=alexcrichton
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

💔 Test failed - auto-win-gnu-32-opt-rustbuild

Contributor

bors commented Sep 30, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 30, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Sep 30, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit ba9e75e has been approved by alexcrichton

Contributor

bors commented Sep 30, 2016

📌 Commit ba9e75e has been approved by alexcrichton

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 30, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Sep 30, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit 57cd2d6 has been approved by alexcrichton

Contributor

bors commented Sep 30, 2016

📌 Commit 57cd2d6 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

⌛️ Testing commit 57cd2d6 with merge 9f0dc1b...

Contributor

bors commented Sep 30, 2016

⌛️ Testing commit 57cd2d6 with merge 9f0dc1b...

bors added a commit that referenced this pull request Sep 30, 2016

Auto merge of #36807 - brson:pal, r=alexcrichton
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

💔 Test failed - auto-mac-64-opt-rustbuild

Contributor

bors commented Sep 30, 2016

💔 Test failed - auto-mac-64-opt-rustbuild

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Sep 30, 2016

Contributor

@bors r+ r=alexcrichton

Contributor

brson commented Sep 30, 2016

@bors r+ r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit 9da0b06 has been approved by alexcrichton

Contributor

bors commented Sep 30, 2016

📌 Commit 9da0b06 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 30, 2016

Contributor

📌 Commit 9da0b06 has been approved by brson

Contributor

bors commented Sep 30, 2016

📌 Commit 9da0b06 has been approved by brson

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

🔒 Merge conflict

Contributor

bors commented Oct 1, 2016

🔒 Merge conflict

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

☔️ The latest upstream changes (presumably #36339) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor

bors commented Oct 1, 2016

☔️ The latest upstream changes (presumably #36339) made this pull request unmergeable. Please resolve the merge conflicts.

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Oct 1, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Oct 1, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

📌 Commit b710344 has been approved by alexcrichton

Contributor

bors commented Oct 1, 2016

📌 Commit b710344 has been approved by alexcrichton

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Oct 1, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Oct 1, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

📌 Commit 1a30e0f has been approved by alexcrichton

Contributor

bors commented Oct 1, 2016

📌 Commit 1a30e0f has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

⌛️ Testing commit 1a30e0f with merge bdded68...

Contributor

bors commented Oct 1, 2016

⌛️ Testing commit 1a30e0f with merge bdded68...

bors added a commit that referenced this pull request Oct 1, 2016

Auto merge of #36807 - brson:pal, r=alexcrichton
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

💔 Test failed - auto-mac-cross-ios-opt

Contributor

bors commented Oct 1, 2016

💔 Test failed - auto-mac-cross-ios-opt

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Oct 1, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Oct 1, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 1, 2016

Contributor

📌 Commit 405c010 has been approved by alexcrichton

Contributor

bors commented Oct 1, 2016

📌 Commit 405c010 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

⌛️ Testing commit 405c010 with merge d97a08e...

Contributor

bors commented Oct 2, 2016

⌛️ Testing commit 405c010 with merge d97a08e...

bors added a commit that referenced this pull request Oct 2, 2016

Auto merge of #36807 - brson:pal, r=alexcrichton
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

💔 Test failed - auto-win-msvc-32-opt

Contributor

bors commented Oct 2, 2016

💔 Test failed - auto-win-msvc-32-opt

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Oct 2, 2016

Contributor

@bors r=alexcrichton

Contributor

brson commented Oct 2, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

📌 Commit 61afff3 has been approved by alexcrichton

Contributor

bors commented Oct 2, 2016

📌 Commit 61afff3 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

⌛️ Testing commit 61afff3 with merge f3add2e...

Contributor

bors commented Oct 2, 2016

⌛️ Testing commit 61afff3 with merge f3add2e...

bors added a commit that referenced this pull request Oct 2, 2016

Auto merge of #36807 - brson:pal, r=alexcrichton
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

💔 Test failed - auto-linux-32-opt

Contributor

bors commented Oct 2, 2016

💔 Test failed - auto-linux-32-opt

brson added some commits Sep 22, 2016

Add a platform-abstraction tidy script
This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.
@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Oct 2, 2016

Contributor

@bors r+

Contributor

brson commented Oct 2, 2016

@bors r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 2, 2016

Contributor

📌 Commit 4d76ac8 has been approved by brson

Contributor

bors commented Oct 2, 2016

📌 Commit 4d76ac8 has been approved by brson

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 3, 2016

Contributor

⌛️ Testing commit 4d76ac8 with merge 144af3e...

Contributor

bors commented Oct 3, 2016

⌛️ Testing commit 4d76ac8 with merge 144af3e...

bors added a commit that referenced this pull request Oct 3, 2016

Auto merge of #36807 - brson:pal, r=brson
Restrict where in the tree platform-specific cfgs may be mentioned

With the ports of Rust never ending, it's important that we keep things tidy. The main thing this PR does is introduce  a new "pal" (platform abstraction layer) tidy check that limits where platform-specific CFGs may appear.

This is intended to maintain existing standards of code organization
in hopes that the standard library will continue to be refactored to
isolate platform-specific bits, making porting easier; where "standard
library" roughly means "all the dependencies of the std and test
crates".

This generally means placing restrictions on where `cfg(unix)`,
`cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
the basic objective being to isolate platform-specific code to the
platform-specific `std::sys` modules, and to the allocation,
unwinding, and libc crates.

Following are the basic rules, though there are currently
exceptions:

- core may not have platform-specific code
- liballoc_system may have platform-specific code
- liballoc_jemalloc may have platform-specific code
- libpanic_abort may have platform-specific code
- libpanic_unwind may have platform-specific code
- other crates in the std facade may not
- std may have platform-specific code in the following places
  - sys/unix/
  - sys/windows/
  - os/

There are plenty of exceptions today though, noted in the whitelist.

The end-state, IMO, is for the standard library to be portable by porting only `std::sys` (possibly extracted to its own crate), an allocator crate, an unwinder crate, and possibly a libc crate (if std depends on it); but that outcome is far off and independent of the utility of enforcing where such code lives today.

cc @rust-lang/libs

@bors bors merged commit 4d76ac8 into rust-lang:master Oct 3, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@bors bors referenced this pull request Oct 3, 2016

Closed

Fix UB in transmute #34181

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