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

Rollup of 8 pull requests #59682

Closed
wants to merge 38 commits into from
Closed

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Apr 3, 2019

Successful merges:

Failed merges:

r? @ghost

matklad and others added 30 commits April 3, 2019 11:41
- libarena
- librustc_allocator
- librustc_borrowck
- librustc_codegen_ssa
- librustc_codegen_utils
- librustc_driver
- librustc_errors
- librustc_incremental
- librustc_metadata
- librustc_passes
- librustc_privacy
- librustc_resolve
- librustc_save_analysis
- librustc_target
- librustc_traits
- libsyntax
- libsyntax_ext
- libsyntax_pos
- libfmt_macros
- librustdoc
This commit removes usage of `Once` from the internal implementation of
time utilities on OSX and Windows. It turns out that we accidentally hit
a deadlock today (rust-lang#59020) via events that look like:

* A thread invokes `park_timeout`
* Internally, only on OSX, `park_timeout` calls `Instant::elapsed`
* Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize
  global timer data
* Inside of `Once`, it attempts to `park`

This means on the same stack frame, when there's contention, we're
calling `park` from inside `park_timeout`, causing a deadlock!

The solution implemented in this commit was to remove usage of `Once`
and instead just do a small dance with atomics. There's no real need we
need to guarantee that the global information is only learned once, only
that it's only *stored* once. This implementation may have multiple
threads invoke `mach_timebase_info`, but only one will store the global
information which will amortize the cost for all other threads.

A similar fix has been applied to windows to be uniform across our
implementations, but looking at the code on Windows no deadlock was
possible. This is purely just a consistency update for Windows and in
theory a slightly leaner implementation.

Closes rust-lang#59020
When using the `rustfix-coverage` flag, some tests currently fail
because they define a different error-format than `json`.

The current implementation crashes when encountering those tests. Since
we don't care about non-json test output when collecting the coverage
data, we handle those tests by returning an empty `Vec` instead.
…li-obk

Internal lints take 2

cc rust-lang#58701
cc rust-lang#49509

TODO: Add `#![warn(internal)]` to crates (and fix violations)

Crates depending on `rustc_data_structures`

- [x] librustc_resolve
- [x] librustc_driver
- [x] librustc_passes
- [x] librustc_metadata
- [x] librustc_interface
- [x] librustc_save_analysis
- [x] librustc_lint
- [x] librustc
- [x] librustc_incremental
- [x] librustc_codegen_utils
- [x] libarena
- [x] librustc_target
- [x] librustc_allocator
- [x] librustc_privacy
- [x] librustc_traits
- [x] librustc_borrowck
- [x] libsyntax
- [x] librustc_codegen_ssa
- [x] libsyntax_ext
- [x] librustc_errors
- [x] librustc_mir
- [x] libsyntax_pos
- [x] librustc_typeck

Crates with `feature(rustc_private)`
Excluding crates, which are already in the list above. Also excluding tools and tests.

- [ ] ~~libstd~~
- [x] libfmt_macros
- [x] librustdoc

r? @oli-obk
Be more direct about borrow contract

I always was confused by the difference between Borrow and AsRef, despite the fact that I've read all available docs at least a dozen of times.

I finally grokked the difference between the two when I realized the Borrow invariant:

> If you implement Borrow, you **must** make sure that Eq, Ord and Hash implementations are equivalent for borrowed and owned data

My problem was that this invariant is not stated explicitly in documentation, and instead some  vague and philosophical notions are used.

So I suggest to mention the requirements of `Borrow` very explicitly: instead of "use Borrow when X and use AsRef when Y", let's phrase this as `Borrow` differs from `AsRef` in `W`, so that's why `Borrow` is for `X` and `AsRef` is for `Y`.

Note that this change could be seen as tightening contract of the Borrow. Let's say Alice has written the following code:

```rust
#[derive(PartialEq, Eq, Hash, PartialOrd, Ord)]
struct Person {
    first_name: String,
    last_name: String,
}

impl Borrow<str> for Person {
      fn borrow(&self) -> &str { self.first_name.as_str() }
}
```

Now Bob uses this `Person` struct, puts it into `HashMap` and tries to look it up using `&str` for the first name. Bob's code naturally fails.

The question is, who is to blame: Alice, who has written the impl, or Bob, who uses the HashMap. If I read the current docs literally, I would say that `Bob` is to blame: `Eq` and `Hash` bounds appear on HashMap, so it is the HashMap which requires that they are consistent. By using a type for which the `Borrow` impl does not yield well-behaved `Eq`, Bob is violating contract of HashMap.

If, as this PR proposes, we unconditionally require that Eq & friends for borrow should be valid, then the blame shifts to Alice, which I think is more reasonable.

closes rust-lang#44868
… r=alexcrichton

Updated the documentation of spin_loop and spin_loop_hint

# Description

- Updated the description of `core::hints::spin_loop`
- Updated the description of `core::async::spin_loop_hint`

Both documentation is rewritten to better reflect when one should prefer using a busy-wait spin-loop (and the `spin_loop` and `spin_loop_hint` functions) over `yield_now`. It also dives a little bit deeper on what the function actually does.

closes rust-lang#55418
…criptions, r=GuillaumeGomez

Updated the environment description in rustc.

# Description

- Updated the "environment" description in the `rustc` man pages

The old wording suggested that all the mentioned flags influenced the output of the compiler,
where this was not the case.

closes rust-lang#59504
Reduce repetition in librustc(_lint) wrt. impl LintPass by using macros

r? @oli-obk
cc @Zoxc
Revert rust-lld place changes

Fixes rust-lang#59661.

Instead of rust-lang#59668 it reverts only failed part.
std: Avoid usage of `Once` in `Instant`

This commit removes usage of `Once` from the internal implementation of
time utilities on OSX and Windows. It turns out that we accidentally hit
a deadlock today (rust-lang#59020) via events that look like:

* A thread invokes `park_timeout`
* Internally, only on OSX, `park_timeout` calls `Instant::elapsed`
* Inside of `Instant::elapsed` on OSX we enter a `Once` to initialize
  global timer data
* Inside of `Once`, it attempts to `park`

This means on the same stack frame, when there's contention, we're
calling `park` from inside `park_timeout`, causing a deadlock!

The solution implemented in this commit was to remove usage of `Once`
and instead just do a small dance with atomics. There's no real need we
need to guarantee that the global information is only learned once, only
that it's only *stored* once. This implementation may have multiple
threads invoke `mach_timebase_info`, but only one will store the global
information which will amortize the cost for all other threads.

A similar fix has been applied to windows to be uniform across our
implementations, but looking at the code on Windows no deadlock was
possible. This is purely just a consistency update for Windows and in
theory a slightly leaner implementation.

Closes rust-lang#59020
…er_error_formats, r=oli-obk

rustfix coverage: Skip UI tests with non-json error-format

When using the `rustfix-coverage` flag, some tests currently fail
because they define a different error-format than `json`.

The current implementation crashes when encountering those tests. Since
we don't care about non-json test output when collecting the coverage
data, we handle those tests by returning an empty `Vec` instead.

r? @oli-obk
@Centril
Copy link
Contributor Author

Centril commented Apr 3, 2019

@bors r+ p=8

@bors
Copy link
Contributor

bors commented Apr 3, 2019

📌 Commit f5f8547 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 3, 2019
@bors
Copy link
Contributor

bors commented Apr 3, 2019

⌛ Testing commit f5f8547 with merge a18409138d0d079a7f44f4da37cf52ea6df1f546...

@rust-highfive
Copy link
Collaborator

The job test-various of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:23:20] ---- [run-pass] run-pass/issue-59020.rs stdout ----
[01:23:20] 
[01:23:20] error: test run failed!
[01:23:20] status: exit code: 101
[01:23:20] command: "/node-v9.2.0-linux-x64/bin/node" "/checkout/src/etc/wasm32-shim.js" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/issue-59020/a.wasm"
[01:23:20] ------------------------------------------
[01:23:20] 
[01:23:20] ------------------------------------------
[01:23:20] stderr:
[01:23:20] stderr:
[01:23:20] ------------------------------------------
[01:23:20] RuntimeError: unreachable
[01:23:20]     at __rust_start_panic (wasm-function[123]:1)
[01:23:20]     at rust_panic (wasm-function[116]:39)
[01:23:20]     at _ZN3std9panicking20rust_panic_with_hook17hf29f358d012886a0E (wasm-function[111]:346)
[01:23:20]     at _ZN3std9panicking18continue_panic_fmt17h2ae2deee0bc74311E (wasm-function[110]:151)
[01:23:20]     at rust_begin_unwind (wasm-function[109]:3)
[01:23:20]     at _ZN4core9panicking9panic_fmt17h4bee53922254e4d1E (wasm-function[143]:80)
[01:23:20]     at _ZN4core6result13unwrap_failed17h0aeaf122db551f96E (wasm-function[11]:122)
[01:23:20]     at _ZN3std6thread5spawn17h94cd7e7a502c98a9E (wasm-function[13]:429)
[01:23:20]     at _ZN11issue_590204main17h83cdb8610bc7a399E (wasm-function[18]:25)
[01:23:20]     at _ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17h91213904f5139701E (wasm-function[3]:25)
[01:23:20]     at _ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17hb3afe3cca1252c5cE (wasm-function[95]:8)
[01:23:20]     at _ZN3std9panicking3try7do_call17h7851f3235948e44fE (wasm-function[108]:20)
[01:23:20]     at __rust_maybe_catch_panic (wasm-function[122]:5)
[01:23:20]     at _ZN3std2rt19lang_start_internal17h73574eba035ece3bE (wasm-function[117]:270)
[01:23:20]     at main (wasm-function[19]:46)
[01:23:20]     at Object.<anonymous> (/checkout/src/etc/wasm32-shim.js:126:20)
[01:23:20]     at Module._compile (module.js:641:30)
[01:23:20]     at Object.Module._extensions..js (module.js:652:10)
[01:23:20]     at Module.load (module.js:560:32)
[01:23:20]     at tryModuleLoad (module.js:503:12)
[01:23:20] ------------------------------------------
[01:23:20] 
[01:23:20] thread '[run-pass] run-pass/issue-59020.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:3425:9
[01:23:20] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
---
[01:23:20] 
[01:23:20] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:516:22
[01:23:20] 
[01:23:20] 
[01:23:20] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/wasm32-unknown-unknown/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-wasm32-unknown-unknown" "--mode" "run-pass" "--target" "wasm32-unknown-unknown" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--nodejs" "/node-v9.2.0-linux-x64/bin/node" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/wasm32-unknown-unknown/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--llvm-version" "8.0.0-rust-1.35.0-dev\n" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:23:20] 
[01:23:20] 
[01:23:20] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test --target wasm32-unknown-unknown src/test/run-make src/test/ui src/test/run-pass src/test/compile-fail src/test/mir-opt src/test/codegen-units src/libcore
[01:23:20] Build completed unsuccessfully in 1:10:46
---
travis_time:end:0ae38131:start=1554334832235306147,finish=1554334832253167827,duration=17861680
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:01512342
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:105941e0
travis_time:start:105941e0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:1857ced4
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Apr 3, 2019

💔 Test failed - checks-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2019
@Centril Centril closed this Apr 3, 2019
@Centril Centril deleted the rollup-ncm5ojo branch April 3, 2019 23:47
@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants