Rollup of 12 pull requests #51214
Closed
Rollup of 12 pull requests #51214
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes rust-lang#35729 According to recommendations in rust-lang#35729 (comment)
…sistent docs order See rust-lang#47115 (comment)
The old text was: "The precise definition is: a type T is Sync if &T is Send."
Since we've also got
```
impl<'a, T> Send for &'a T
where
T: Sync + ?Sized,
```
I purpose we can change the `if` to `if and only if` to make it more precise.
Fixes rust-lang#47275. These two macros are similar, but different, and could do with documentation links to each other.
It now details why using compile_error!() is different from just not having the final macro_rules!() branch.
This commit adds a new section to the Documentation Test docs, which briefly mentions indented code blocks, and links to the CommonMark specification for both. I’m not sure about saying "fenced code blocks the more popular choice in the Rust community” because it seems like I’m speaking for everyone, but I can’t think of a better way to phrase it!
If a struct pattern has a field error return an error.
…are now exactly the same as that produced by AST borrowck. Bravo!
It's never used in a meaningful way.
We already got the open file descriptor at this point. Don't make the kernel resolve the path again.
This reverts commit 068fc44.
Because they are small and hot.
|
r? @aidanhs (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ p=10 |
|
📌 Commit be32243 has been approved by |
bors
added
S-waiting-on-bors
|
🔒 Merge conflict |
bors
added
S-waiting-on-author
std::fs::DirEntry.metadata(): use fstatat instead of lstat when possible When reading a directory with `read_dir`, querying metadata for a resulting `DirEntry` is done by building the whole path and then `lstat`ing it, which requires the kernel to resolve the whole path. Instead, one can use the file descriptor to the enumerated directory and use `fstatat`. This make the resolving step unnecessary. This PR implements using `fstatat` on linux, android and emscripten. ## Compatibility across targets `fstatat` is POSIX. * Linux >= 2.6.19 according to https://linux.die.net/man/2/fstatat * android according to https://android.googlesource.com/platform/bionic/+/master/libc/libc.map.txt#392 * emscripten according to https://github.com/kripken/emscripten/blob/7f89560101843198787530731f40a65288f6f15f/system/include/libc/sys/stat.h#L76 The man page says "A similar system call exists on Solaris." but I haven't found it. ## Compatibility with old platforms This was introduced with glibc 2.4 according to the man page. The only information I could find about the minimal version of glibc rust must support is this discussion https://internals.rust-lang.org/t/bumping-glibc-requirements-for-the-rust-toolchain/5111/10 The conclusion, if I understand correctly, is that currently rust supports glibc >= 2.3.4 but the "real" requirement is Centos 5 with glibc 2.5. This PR would make the minimal version 2.4, so this should be fine. ## Benefit I did the following silly benchmark: ```rust use std::io; use std::fs; use std::os::linux::fs::MetadataExt; use std::time::Instant; fn main() -> Result<(), io::Error> { let mut n = 0; let mut size = 0; let start = Instant::now(); for entry in fs::read_dir("/nix/store/.links")? { let entry = entry?; let stat = entry.metadata()?; size += stat.st_size(); n+=1; } println!("{} files, size {}, time {:?}", n, size, Instant::now().duration_since(start)); Ok(()) } ``` On warm cache, with current rust nightly: ``` 1014099 files, size 76895290022, time Duration { secs: 2, nanos: 65832118 } ``` (between 2.1 and 2.9 seconds usually) With this PR: ``` 1014099 files, size 76895290022, time Duration { secs: 1, nanos: 581662953 } ``` (1.5 to 1.6 seconds usually). approximately 40% faster :) On cold cache there is not much to gain because path lookup (which we spare) would have been a cache hit: Before ``` 1014099 files, size 76895290022, time Duration { secs: 391, nanos: 739874992 } ``` After ``` 1014099 files, size 76895290022, time Duration { secs: 388, nanos: 431567396 } ``` ## Testing The tests were run on linux `x86_64` ``` python x.py test src/tools/tidy ./x.py test src/libstd ``` and the above benchmark. I did not test any other target.
Update build instructions It get stuck at the cloning step. `./x.py build ` Updating only changed submodules Updating submodule src/llvm Submodule 'src/llvm' (https://github.com/rust-lang/llvm.git) registered for path 'src/llvm' Cloning into '/home/username/rust/src/llvm'...
…uillaumeGomez Add doc link from discriminant struct to function. None
typeck: Do not pass the field check on field error If a struct pattern has a field error return an error. Fixes: rust-lang#51102
Stabilize SliceIndex trait. CC rust-lang#35729 According to recommendations in rust-lang#35729 (comment)
…dtolnay Move slice::exact_chunks directly above exact_chunks_mut for more con… …sistent docs order See rust-lang#47115 (comment)
… r=GuillaumeGomez Link panic and compile_error docs This adds documentation links between `panic!()` and `compile_error!()` as per rust-lang#47275, which points out that they’re similar. It also adds a sentence to the `compile_error()` docs I thought could be added.
Mention spec and indented blocks in doctest docs Fixes rust-lang#49717. This commit adds a new section to the Documentation Test docs, which briefly mentions indented code blocks, and links to the CommonMark specification for both. I’m not sure about saying "fenced code blocks the more popular choice in the Rust community” because it seems like I’m speaking for everyone, but I can’t think of a better way to phrase it!
…derr-files, r=oli-obk Remove two redundant .nll.stderr files It turns out that the diagnostics generated from NLL for these cases are now exactly the same as that produced by AST borrowck, and thus we can just fallback on those `.stderr` files that already exist for AST-borrowck. Bravo! (it is a good idea to remove these files, because it slightly reduces the amount of time humans will spend reviewing the .nll.stderr fileset...) ((it *might* be worthwhile trying to change the `compiletest` code to even issue a warning when two such files have equivalent contents... but I am not going so far as to try to implement that right now...))
…lwoerister Two minor `obligation_forest` tweaks. Pretty minimal improvements, but improvements nonetheless.
|
@bors r+ |
|
📌 Commit 7bf60b7 has been approved by |
bors
added
S-waiting-on-bors
fs: copy: Use File::set_permissions instead of fs::set_permissions We already got the open file descriptor at this point. Don't make the kernel resolve the path again.
The old text was: "The precise definition is: a type `T` is `Sync` if `&T` is Send."
Since we've also got
```
impl<'a, T> Send for &'a T
where
T: Sync + ?Sized,
```
I purpose we can change the `if` to `if and only if` to make it more precise.
|
📌 Commit b7b7b25 has been approved by |
bors
added a commit
that referenced
this pull request
May 30, 2018
Rollup of 12 pull requests Successful merges: - #51050 (std::fs::DirEntry.metadata(): use fstatat instead of lstat when possible) - #51123 (Update build instructions) - #51127 (Add doc link from discriminant struct to function.) - #51146 (typeck: Do not pass the field check on field error) - #51147 (Stabilize SliceIndex trait.) - #51151 (Move slice::exact_chunks directly above exact_chunks_mut for more con…) - #51152 (Replace `if` with `if and only if` in the definition dox of `Sync`) - #51153 (Link panic and compile_error docs) - #51158 (Mention spec and indented blocks in doctest docs) - #51186 (Remove two redundant .nll.stderr files) - #51203 (Two minor `obligation_forest` tweaks.) - #51213 (fs: copy: Use File::set_permissions instead of fs::set_permissions) Failed merges:
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
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 |
1 similar comment
|
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
ifwithif and only ifin the definition dox ofSync#51152 (Replaceifwithif and only ifin the definition dox ofSync)obligation_foresttweaks. #51203 (Two minorobligation_foresttweaks.)Failed merges: