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.

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

More refactoring to obey platform abstraction lint #36948

Merged
merged 10 commits into from Nov 2, 2016

Conversation

Projects
None yet
5 participants
@brson
Copy link
Contributor

brson commented Oct 4, 2016

The most interesting things here are moving std/sys/common to std/sys_common, and std/num/{f32,f64}.rs to std/{f32,f64}.rs, and adding more documentation to std/lib.rs.

r? @alexcrichton

rtabort!(concat!("assertion failed: ", stringify!($e)))
}
})
}

This comment has been minimized.

@brson

brson Oct 4, 2016

Author Contributor

This macro is no longer used anywhere.

This comment has been minimized.

@alexcrichton

alexcrichton Oct 4, 2016

Member

🎺🎺

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Oct 4, 2016

A lot of this is just general cleanup...

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 4, 2016

src/libstd/num/f64.rs → src/libstd/f64.rs

How dare you!


// Turn warnings into errors, but only after stage0, where it can be useful for
// code to emit warnings during language transitions
#![cfg_attr(not(stage0), deny(warnings))]

This comment has been minimized.

@alexcrichton

alexcrichton Oct 4, 2016

Member

If you're really ambitious, I've been meaning to remove all the cfg_attr here and just leave deny(warnings)

@@ -425,46 +403,62 @@ pub use core::u16;
pub use core::u32;
#[stable(feature = "rust1", since = "1.0.0")]
pub use core::u64;
#[stable(feature = "rust1", since = "1.0.0")]
pub use alloc::boxed;

This comment has been minimized.

@alexcrichton

alexcrichton Oct 4, 2016

Member

I think the ordering here affects the ordering in rustdoc, right? (just confirming you want to change that too)

This comment has been minimized.

@brson

brson Oct 4, 2016

Author Contributor

It does not appear so to me. rustdoc seems to be alphabetizing everything.

@@ -184,6 +184,10 @@ macro_rules! assert_approx_eq {
})
}

macro_rules! rtabort {

This comment has been minimized.

@alexcrichton

alexcrichton Oct 4, 2016

Member

aww I was hoping we could keep this contained within the sys module

This comment has been minimized.

@brson

brson Oct 4, 2016

Author Contributor

I'll see if I can put it there. I don't recall why I moved it, presumably just because it didn't work with my prefered module declaration order in lib.rs.

#[doc(hidden)]
#[cfg(not(target_thread_local))]

This comment has been minimized.

@alexcrichton

alexcrichton Oct 4, 2016

Member

I think we'll want to always compile in this module. The theory behind this is that we compile the standard library with OS-based TLS (e.g. this module) but then consumers of the standard library can use the "fast TLS". The primary motivation here is OSX 10.6 and 10.7+, where 10.6 has slow TLS only but 10.7 has fast. We could in theory compile a 10.6 standard library but everyone else would have fast TLS.

This comment has been minimized.

@brson

brson Oct 4, 2016

Author Contributor

OK.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 4, 2016

moving std/sys/common to std/sys_common

I kinda liked having everything contained in one nice "sys" directory, but I guess not enough to go against conventions of filesystem hierarchies and module structures.

@brson brson force-pushed the brson:sys branch from bc4c1d4 to ae10558 Oct 4, 2016

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Oct 4, 2016

Updated to leave os-specific TLS key in with documentation for why, move rtabort back to sys_common.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 4, 2016

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 4, 2016

📌 Commit ae10558 has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 6, 2016

⌛️ Testing commit ae10558 with merge d3b75bb...

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

Auto merge of #36948 - brson:sys, r=alexcrichton
More refactoring to obey platform abstraction lint

The most interesting things here are moving `std/sys/common` to `std/sys_common`, and `std/num/{f32,f64}.rs` to `std/{f32,f64}.rs`, and adding more documentation to `std/lib.rs`.

r? @alexcrichton
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 6, 2016

💥 Test timed out

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 6, 2016

@bors: retry

  • ??
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 6, 2016

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

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 10, 2016

@bors: retry

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 13, 2016

🔒 Merge conflict

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 31, 2016

ping @brson (for an update)

brson added some commits Oct 1, 2016

std: Flatten the num directory to reflect the module layout
This makes it dissimilar to how core is structured on disk, but
more predictable on its own.

@brson brson force-pushed the brson:sys branch from ae10558 to 6135cbc Nov 1, 2016

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Nov 1, 2016

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 1, 2016

📌 Commit 6135cbc has been approved by brson

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 2, 2016

⌛️ Testing commit 6135cbc with merge fc67cad...

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

Auto merge of #36948 - brson:sys, r=brson
More refactoring to obey platform abstraction lint

The most interesting things here are moving `std/sys/common` to `std/sys_common`, and `std/num/{f32,f64}.rs` to `std/{f32,f64}.rs`, and adding more documentation to `std/lib.rs`.

r? @alexcrichton
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 2, 2016

💔 Test failed - auto-win-gnu-64-opt

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Nov 2, 2016

@bors: retry

On Wednesday, November 2, 2016, bors notifications@github.com wrote:

💔 Test failed - auto-win-gnu-64-opt
https://buildbot.rust-lang.org/builders/auto-win-gnu-64-opt/builds/6005


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#36948 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95PUMAp0IYQNuNyPDU8pI9h-DwlSBks5q6IQPgaJpZM4KNL4c
.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Nov 2, 2016

⌛️ Testing commit 6135cbc with merge 0ca9967...

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

Auto merge of #36948 - brson:sys, r=brson
More refactoring to obey platform abstraction lint

The most interesting things here are moving `std/sys/common` to `std/sys_common`, and `std/num/{f32,f64}.rs` to `std/{f32,f64}.rs`, and adding more documentation to `std/lib.rs`.

r? @alexcrichton

@bors bors merged commit 6135cbc into rust-lang:master Nov 2, 2016

2 checks passed

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

This comment has been minimized.

Copy link
Contributor

benaryorg commented on 6d54cd4 Mar 25, 2018

Sorry to dig up this rather old commit, but did this actually drop the distinction of Windows 8k buffers and Unix 64k buffers and made everything 8k?

This comment has been minimized.

Copy link

sourcejedi replied Apr 22, 2018

@benaryorg commit 8128817 goes from 64k to 8k, that might be what you're thinking of. I'm not sure where the idea of a different value on Windows comes from, unless that was the behaviour even earlier when libuv was used.

This comment has been minimized.

Copy link
Contributor

benaryorg replied Apr 23, 2018

The distinction came from Windows not handling the 64k buffers well, hence they choose to set it so something less on Windows.
But seeing it being set to 8k on both of 'em, this doesn't matter now anyway.
Thanks for your effort though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.