Yuki Okushi [Sun, 10 Nov 2019 00:27:10 +0000 (09:27 +0900)]
Rollup merge of #65719 - pitdicker:refactor_sync_once, r=Amanieu
Refactor sync::Once
`std::sync::Once` contains some tricky code to park and unpark waiting threads. [once_cell](https://github.com/matklad/once_cell) has very similar code copied from here. I tried to add more comments and refactor the code to make it more readable (at least in my opinion). My PR to `once_cell` was rejected, because it is an advantage to remain close to the implementation in std, and because I made a mess of the atomic orderings. So now a PR here, with similar changes to `std::sync::Once`!
The initial goal was to see if there is some way to detect reentrant initialization instead of deadlocking. No luck there yet, but you first have to understand and document the complexities of the existing code :smile:.
*Maybe not this entire PR will be acceptable, but I hope at least some of the commits can be useful.*
Individual commits:
#### Rename state to state_and_queue
Just a more accurate description, although a bit wordy. It helped me on a first read through the code, where before `state` was use to encode a pointer in to nodes of a linked list.
#### Simplify loop conditions in RUNNING and add comments
In the relevant loop there are two things to be careful about:
- make sure not to enqueue the current thread only while still RUNNING, otherwise we will never be woken up (the status may have changed while trying to enqueue this thread).
- pick up if another thread just replaced the head of the linked list.
Because the first check was part of the condition of the while loop, the rest of the parking code also had to live in that loop. It took me a while to get the subtlety here, and it should now be clearer.
Also call out that we really have to wait until signaled, otherwise we leave a dangling reference.
#### Don't mutate waiter nodes
Previously while waking up other threads the managing thread would `take()` out the `Thread` struct and use that to unpark the other thread. It is just as easy to clone it, just 24 bytes. This way `Waiter.thread` does not need an `Option`, `Waiter.next` does not need to be a mutable pointer, and there is less data that needs to be synchronised by later atomic operations.
#### Turn Finish into WaiterQueue
In my opinion these changes make it just a bit more clear what is going on with the thread parking stuff.
#### Move thread parking to a seperate function
Maybe controversial, but with this last commit all the thread parking stuff has a reasonably clean seperation from the state changes in `Once`. This is arguably the trickier part of `Once`, compared to the loop in `call_inner`. It may make it easier to reuse parts of this code (see https://github.com/rust-lang/rfcs/pull/2788#discussion_r336729695). Not sure if that ever becomes a reality though.
#### Reduce the amount of comments in call_inner
With the changes from the previous commits, the code pretty much speaks for itself, and the amount of comments is hurting readability a bit.
#### Use more precise atomic orderings
Now the hard one. This is the one change that is not anything but a pure refactor or change of comments.
I have a dislike for using `SeqCst` everywhere, because it hides what the atomics are supposed to do. the rationale was:
> This cold path uses SeqCst consistently because the performance difference really does not matter there, and SeqCst minimizes the chances of something going wrong.
But in my opinion, having the proper orderings and some explanation helps to understand what is going on. My rationale for the used orderings (also included as comment):
When running `Once` we deal with multiple atomics: `Once.state_and_queue` and an unknown number of `Waiter.signaled`.
* `state_and_queue` is used (1) as a state flag, (2) for synchronizing the data that is the result of the `Once`, and (3) for synchronizing `Waiter` nodes.
- At the end of the `call_inner` function we have to make sure the result of the `Once` is acquired. So every load which can be the only one to load COMPLETED must have at least Acquire ordering, which means all three of them.
- `WaiterQueue::Drop` is the only place that may store COMPLETED, and must do so with Release ordering to make result available.
- `wait` inserts `Waiter` nodes as a pointer in `state_and_queue`, and needs to make the nodes available with Release ordering. The load in its `compare_and_swap` can be Relaxed because it only has to compare the atomic, not to read other data.
- `WaiterQueue::Drop` must see the `Waiter` nodes, so it must load `state_and_queue` with Acquire ordering.
- There is just one store where `state_and_queue` is used only as a state flag, without having to synchronize data: switching the state from INCOMPLETE to RUNNING in `call_inner`. This store can be Relaxed, but the read has to be Acquire because of the requirements mentioned above.
* `Waiter.signaled` is both used as a flag, and to protect a field with interior mutability in `Waiter`. `Waiter.thread` is changed in `WaiterQueue::Drop` which then sets `signaled` with Release ordering. After `wait` loads `signaled` with Acquire and sees it is true, it needs to see the changes to drop the `Waiter` struct correctly.
* There is one place where the two atomics `Once.state_and_queue` and `Waiter.signaled` come together, and might be reordered by the compiler or processor. Because both use Aquire ordering such a reordering is not allowed, so no need for SeqCst.
bors [Sat, 9 Nov 2019 09:33:13 +0000 (09:33 +0000)]
Auto merge of #66242 - Centril:rollup-h73ztr1, r=Centril
Rollup of 6 pull requests
Successful merges:
- #65949 (Move promotion into its own pass)
- #65994 (Point at where clauses where the associated item was restricted)
- #66050 (Fix C aggregate-passing ABI on powerpc)
- #66134 (Point at formatting descriptor string when it is invalid)
- #66172 (Stabilize @file command line arguments)
- #66226 (add link to unstable book for asm! macro)
Rollup merge of #66134 - estebank:unknown-formatting-trait, r=nikomatsakis
Point at formatting descriptor string when it is invalid
When a formatting string contains an invalid descriptor, point at it
instead of the argument:
```
error: unknown format trait `foo`
--> $DIR/ifmt-bad-arg.rs:86:17
|
LL | println!("{:foo}", 1);
| ^^^
|
= note: the only appropriate formatting traits are:
- ``, which uses the `Display` trait
- `?`, which uses the `Debug` trait
- `e`, which uses the `LowerExp` trait
- `E`, which uses the `UpperExp` trait
- `o`, which uses the `Octal` trait
- `p`, which uses the `Pointer` trait
- `b`, which uses the `Binary` trait
- `x`, which uses the `LowerHex` trait
- `X`, which uses the `UpperHex` trait
```
Rollup merge of #66050 - smaeul:patch/powerpc-abi-2, r=eddyb
Fix C aggregate-passing ABI on powerpc
The existing code (which looks like it was copied from MIPS) passes
aggregates by value in registers. This is wrong. According to the SVR4
powerpc psABI, all aggregates are passed indirectly.
See #64259 for more discussion, which addresses the ABI for the special
case of ZSTs (empty structs).
Rollup merge of #65949 - ecstatic-morse:promote-only-pass, r=eddyb
Move promotion into its own pass
**edited**
This adds a `PromoteTemps` pass, which runs after the old `QualifyAndPromoteConsts` pass, that *only* does promotion (no const-checking). Everything related to promotion has been removed from `QualifyAndPromoteConstants`: it no longer even visits the body of a non-const `fn`.
As a result we no longer need to keep the `BitSet` of promotable locals that was returned by `mir_const_qualif`. Rvalue-static promotion in a `const` is now done in `promote_consts`, and it operates on a set of `Candidate`s instead. This will allow me–in a later PR–to create promoted MIR fragments for `const`s when necessary, which could resolve some shortcomings of the current approach (removing `StorageDead`).
bors [Sat, 9 Nov 2019 05:57:14 +0000 (05:57 +0000)]
Auto merge of #65879 - ohadravid:stabilize-re-rebalance-coherence, r=nikomatsakis
Stabilize the `re_rebalance_coherence` feature
This PR stabilizes [RFC 2451](https://rust-lang.github.io/rfcs/2451-re-rebalancing-coherence.html), re-rebalance coherence.
Changes include removing the attribute from tests which tested both the old and new behavior, moving the feature to `accepted` and removing the old logic.
I'll also open a [PR](https://github.com/rust-lang-nursery/reference/pull/703) against the reference, updating it with the content of the RFC.
bors [Fri, 8 Nov 2019 23:28:06 +0000 (23:28 +0000)]
Auto merge of #66194 - alexcrichton:update-clang, r=Mark-Simulacrum
Update Clang & build MSVC LLVM with it
This is a general update of our builders to Clang 9, and then it also attempts to tackle a bit of #66192 by building LLVM for rustc with Clang, not with the system `cl.exe` on MSVC.
Dylan MacKenzie [Mon, 28 Oct 2019 22:21:53 +0000 (15:21 -0700)]
Bless tests now that we do promotion if `min_const_fn` fails
We bailed out of `QualifyAndPromoteConsts` immediately if the
`min_const_fn` checks failed, which sometimes resulted in additional,
spurious errors since promotion was skipped.
We now do promotion in a completely separate pass, so this is no longer
an issue.
bors [Fri, 8 Nov 2019 15:52:14 +0000 (15:52 +0000)]
Auto merge of #66225 - Centril:rollup-it0t5tk, r=Centril
Rollup of 5 pull requests
Successful merges:
- #65785 (Transition future compat lints to {ERROR, DENY} - Take 2)
- #66007 (Remove "here" from "expected one of X here")
- #66043 (rename Memory::get methods to get_raw to indicate their unchecked nature)
- #66154 (miri: Rename to_{u,i}size to to_machine_{u,i}size)
- #66188 (`MethodSig` -> `FnSig` & Use it in `ItemKind::Fn`)
Rollup merge of #66043 - RalfJung:memory-get-raw, r=cramertj
rename Memory::get methods to get_raw to indicate their unchecked nature
Some recent Miri PRs started using these methods when they should not; this should discourage their use.
In fact we could make these methods private to the `interp` module as far as Miri is concerned -- with the exception of the `uninit` intrinsic which will hopefully go away soon. @bjorn3 @oli-obk does priroda need these methods? It would be great to be able to seal them away.
bors [Fri, 8 Nov 2019 07:55:53 +0000 (07:55 +0000)]
Auto merge of #66066 - ecstatic-morse:remove-promotion-from-qualify-consts, r=eddyb
Remove promotion candidate gathering and checking from `qualify_consts.rs`
This makes promotion candidate gathering and checking the exclusive domain of `promote_consts`, but the `QualifyAndPromoteConsts` pass is still responsible for both const-checking and creating promoted MIR fragments.
This should not be merged until the beta branches on Nov. 5.
bors [Fri, 8 Nov 2019 04:44:23 +0000 (04:44 +0000)]
Auto merge of #66208 - JohnTitor:rollup-2umgjer, r=JohnTitor
Rollup of 8 pull requests
Successful merges:
- #65554 (Enhance the documentation of BufReader for potential data loss)
- #65580 (Add `MaybeUninit` methods `uninit_array`, `slice_get_ref`, `slice_get_mut`)
- #66049 (consistent handling of missing sysroot spans)
- #66056 (rustc_metadata: Some reorganization of the module structure)
- #66123 (No more hidden elements)
- #66157 (Improve math log documentation examples)
- #66165 (Ignore these tests ,since the called commands doesn't exist in VxWorks)
- #66190 (rustc_target: inline abi::FloatTy into abi::Primitive.)
Failed merges:
- #66188 (`MethodSig` -> `FnSig` & Use it in `ItemKind::Fn`)
Yuki Okushi [Fri, 8 Nov 2019 04:42:24 +0000 (13:42 +0900)]
Rollup merge of #66190 - eddyb:primflt, r=Centril
rustc_target: inline abi::FloatTy into abi::Primitive.
This effectively undoes a small part of @oli-obk's #50967, now that the rest of the compiler doesn't use the `FloatTy` definition from `rustc_target`, post-#65884.
Yuki Okushi [Fri, 8 Nov 2019 04:42:21 +0000 (13:42 +0900)]
Rollup merge of #66157 - srinivasreddy:improv, r=alexcrichton
Improve math log documentation examples
using 2.0.log(2.0) in examples does not make it clear which is the base and number. This example makes it clear for programmers who take a glance at the example by following the calculation. It is more intuitive, and eliminates the need for executing the example in the playground.
Yuki Okushi [Fri, 8 Nov 2019 04:42:17 +0000 (13:42 +0900)]
Rollup merge of #66056 - petrochenkov:metapriv, r=eddyb
rustc_metadata: Some reorganization of the module structure
The new structure of `rustc_metadata` (or rather its parts affected by the refactoring) is
```
├── lib.rs
└── rmeta
├── decoder
│ └── cstore_impl.rs
├── decoder.rs
├── encoder.rs
├── mod.rs
└── table.rs
```
(`schema` is renamed to `rmeta`.)
The code inside `rmeta` is pretty self-contained, so we can now privatize almost everything in this module instead of using `pub(crate)` which was necessary when all these modules accessed their neighbors in the old flat structure.
`encoder` and `decoder` work with structures defined by `rmeta`.
`table` is a part of `rmeta`.
`cstore_impl` actively uses decoder methods and exposes their results through very few public methods (`provide` and `_untracked` methods).
Yuki Okushi [Fri, 8 Nov 2019 04:42:16 +0000 (13:42 +0900)]
Rollup merge of #66049 - RalfJung:missing-spans, r=alexcrichton
consistent handling of missing sysroot spans
Due to https://github.com/rust-lang/rust/issues/53081, sysroot spans (pointing to code in libcore/libstd/...) fails to print on some x86 runners. This consolidates the ignore directives for that and references the relevant issue.
I also did that for the generated derive-error-span tests -- but there the script and the tests were not entirely in sync any more since https://github.com/rust-lang/rust/pull/64151. Cc @estebank @varkor
Eventually these will hopefully become the idiomatic way to work with partially-initialized stack buffers.
All methods are unstable. Note that `uninit_array` takes a type-level `const usize` parameter, so it is blocked (at least in its current form) on const generics.
Example:
```rust
use std::mem::MaybeUninit;
let input = b"Foo";
let f = u8::to_ascii_uppercase;
let mut buffer: [MaybeUninit<u8>; 32] = MaybeUninit::uninit_array();
let vec;
let output = if let Some(buffer) = buffer.get_mut(..input.len()) {
buffer.iter_mut().zip(input).for_each(|(a, b)| { a.write(f(b)); });
unsafe { MaybeUninit::slice_get_ref(buffer) }
} else {
vec = input.iter().map(f).collect::<Vec<u8>>();
&vec
};
Yuki Okushi [Fri, 8 Nov 2019 04:42:12 +0000 (13:42 +0900)]
Rollup merge of #65554 - gliderkite:bufreader-doc-enhance, r=KodrAus
Enhance the documentation of BufReader for potential data loss
This is (IMO) and enhancement of the `std::io::BufReader` documentation, that aims to highlight how relatively easy is to end up with data loss when improperly using an instance of this class.
This is following the issue I had figuring out why my application was loosing data, because I focused my attention on the word *multiple instances* of `BufReader` in its `struct` documentation, even if I ever only had one instance.
Link to the issue: https://github.com/tokio-rs/tokio/issues/1662
bors [Fri, 8 Nov 2019 01:15:50 +0000 (01:15 +0000)]
Auto merge of #64882 - ehuss:stabilize-bare-extern, r=eddyb
Stabilize --extern flag without a path.
This stabilizes the `--extern` flag without a path, implemented in #54116.
This flag is used to add a crate that may be found in the search path to the extern prelude. The intent of stabilizing this now is to change Cargo to emit this flag for `proc_macro` when building a proc-macro crate. This will allow the ability to elide `extern crate proc_macro;` for proc-macros, one of the few places where it is still necessary.
It is intended that Cargo may also use this flag for other cases in the future as part of the [std-aware work](https://github.com/rust-lang/wg-cargo-std-aware/). There will likely be some kind of syntax where users may declare dependencies on other crates (such as `alloc`), and Cargo will use this flag so that they may be used like any other crate. At this time there are no short-term plans to use it for anything other than proc-macro.
This will not help for non-proc-macro crates that use `proc_macro`, which I believe is not too common?
An alternate approach for proc-macro is to use the `meta` crate, but from my inquiries there doesn't appear to be anyone interested in pushing that forward. The `meta` crate also doesn't help with things like `alloc` or `test`.
bors [Thu, 7 Nov 2019 22:02:41 +0000 (22:02 +0000)]
Auto merge of #66189 - Centril:rollup-3bsf45s, r=Centril
Rollup of 5 pull requests
Successful merges:
- #63793 (Have tidy ensure that we document all `unsafe` blocks in libcore)
- #64696 ([rustdoc] add sub settings)
- #65916 (syntax: move stuff around)
- #66087 (Update some build-pass ui tests to use check-pass where applicable)
- #66182 (invalid_value lint: fix help text)
Alex Crichton [Thu, 7 Nov 2019 17:25:06 +0000 (09:25 -0800)]
Attempt to fix *.yml confguration on Azure
Currently the `RUST_CONFIGURE_ARGS` variable apparently has a trailing
newline at the end of it due to the way it's configured in yaml. This
causes issues with MSVC's `install-clang.sh` step where the way the bash
syntax works out means that we drop the arg we're trying to add and it
doesn't actually get added!
The hopeful fix here is to tweak how we specify the yaml syntax to not
have a trailing newline, we'll see what CI says about this...
Rollup merge of #66182 - RalfJung:invalid-value, r=Centril
invalid_value lint: fix help text
Now that we also warn about `MaybUninit::uninit().assume_init()`, just telling people "use `MaybeUninit`" isn't always sufficient. And anyway this seems like an important enough point to mention it here.
Rollup merge of #64696 - GuillaumeGomez:rustdoc-sub-settings, r=kinnison
[rustdoc] add sub settings
This PR is to give a finer control over what types are automatically expanded or not as well as the possibility to add sub-settings in the settings page.
Rollup merge of #66017 - LukasKalbertodt:array-into-iter-lint, r=matthewjasper
Add future incompatibility lint for `array.into_iter()`
This is for #65819. This lint warns when calling `into_iter` on an array directly. That's because today the method call resolves to `<&[T] as IntoIterator>::into_iter` but that would change when adding `IntoIterator` impls for arrays. This problem is discussed in detail in #65819.
We still haven't decided how to proceed exactly, but it seems like adding a lint is a good idea regardless?
Also: this is the first time I implement a lint, so there are probably a lot of things I can improve. I used a different strategy than @scottmcm describes [here](https://github.com/rust-lang/rust/pull/65819#issuecomment-548667847) since I already started implementing this before they commented.
### TODO
- [x] Decide if we want this lint -> apparently [we want](https://github.com/rust-lang/rust/pull/65819#issuecomment-548964818)
- [x] Open a lint-tracking-issue and add the correct issue number in the code -> https://github.com/rust-lang/rust/issues/66145
Rollup merge of #65974 - Centril:matcher-friendly-gating, r=petrochenkov
A scheme for more macro-matcher friendly pre-expansion gating
Pre-expansion gating will now avoid gating macro matchers that did not result in `Success(...)`. That is, the following is now OK despite `box 42` being a valid `expr` and that form being pre-expansion gated:
```rust
macro_rules! m {
($e:expr) => { 0 }; // This fails on the input below due to `, foo`.
(box $e:expr, foo) => { 1 }; // Successful matcher, we should get `2`.
}
Rollup merge of #65884 - Centril:non-hardcoded-abis, r=petrochenkov
syntax: ABI-oblivious grammar
This PR has the following effects:
1. `extern $lit` is now legal where `$lit:literal` and `$lit` is substituted for a string literal.
2. `extern "abi_that_does_not_exist"` is now *syntactically* legal whereas before, the set of ABI strings was hard-coded into the grammar of the language. With this PR, the set of ABIs are instead validated and translated during lowering. That seems more appropriate.
3. `ast::FloatTy` is now distinct from `rustc_target::abi::FloatTy`. The former is used substantially more and the translation between them is only necessary in a single place.
4. As a result of 2-3, libsyntax no longer depends on librustc_target, which should improve pipe-lining somewhat.
cc @rust-lang/lang -- the points 1-2 slightly change the definition of the language but in a way which seems consistent with our general principles (in particular wrt. the discussions of turning things into semantic errors). I expect this to be uncontroversial but it's worth letting y'all know. :)
Use structured suggestions for missing associated items
When encountering an `impl` that is missing associated items required by its `trait`, use structured suggestions at an appropriate place in the `impl`.
Rollup merge of #59789 - eddyb:typeck-reverts, r=nikomatsakis
Revert two unapproved changes to rustc_typeck.
There was a breakdown in process (https://github.com/rust-lang/rust/pull/59004#issuecomment-477600735, https://github.com/rust-lang/rust/pull/58894#discussion_r272795560) and two changes were made to `rustc_typeck`'s "collect" queries, for rustdoc, that were neither needed *nor* correct.
I'm reverting them here, and will fix up rustdoc *somehow*, if necessary.
cc @rust-lang/compiler How do we ensure this doesn't happen again?