-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Rollup of 6 pull requests #142432
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 6 pull requests #142432
Conversation
Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field.
- Rewords existing Considerations section on `fetch_update` and friends to make clear that the limitations are inherent to an implementation based on any CAS operation, rather than the weak version of `compare_exchange` in particular - Add Considerations to `compare_exchange` and `compare_exchange_weak` which details similar considerations and when they may be relevant.
…e, r=dtolnay Added `Clone` implementation for `ChunkBy` Added `Clone` implementation for `ChunkBy` Closes rust-lang#137969.
refactor `AttributeGate` and `rustc_attr!` to emit notes during feature checking First commit changes the following: - `AttributeGate ` from an enum with (four) tuple fields to (five) named fields - adds a `notes` fields that is emitted as notes in the `PostExpansionVisitor` pass - removes the `this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date` note if the feature gate is `rustc_attrs`. - various phrasing changes and touchups - and finally, the reason why I went down this path to begin with: tell people they can use the diagnostic namespace when they hit the rustc_on_unimplemented feature gate 🙈 Second commit removes unused machinery for deprecated attributes
…errors Add `ParseMode::Diagnostic` and fix multiline spans in diagnostic attribute lints Best viewed commit by commit. The first commit is a test, the commits following that are small refactors to `rustc_parse_format`. Originally I wanted to do a much larger change (doing these smaller fixes first would have that made easier to review), but ended up doing something else instead. An observable change from this is that the diagnostic attribute no longer tries to parse align/fill/width/etc parameters. For an example (see also test changes), a string like `"{Self:!}"` no longer says "missing '}'", instead it says that format parameters are not allowed. It'll now also format the string as if the user wrote just `"{Self}"`
…traviscross Specify that "option-like" enums must be `#[repr(Rust)]` to be ABI-compatible with their non-1ZST field. Add that the enum must be `#[repr(Rust)]` and not `#[repr(packed)]` or `#[repr(align)]` in order to be ABI-compatible with its null-pointer-optimized field. The specific rules here were decided on here: rust-lang#130628 (comment) but `repr` was not mentioned. In practice, only `#[repr(Rust)]` (or no `repr` attribute, which is equivalent) works for this, so add that to the docs. ----- Restrict to `#[repr(Rust)]` only, since: * `#[repr(C)]` and the primitive representations (`#[repr(u8)]` etc) definitely disqualify the enum from NPO, since they have defined layouts that store the tag separately to the payload. * `#[repr(transparent)]` enums are covered two bullet points above this (line 1830), and cannot have multiple variants, so would fail the "The enum has exactly two variants" requirement anyway. As for `#[repr(align)]`: my current wording that it is completely disallowed may be too strong: it seems like `#[repr(align(<= alignment of T))] enum Foo { X, Y(T) }` currently does still have the same ABI as `T` in practice, though this may not be something we want to promise. (`#[repr(align(> alignment of T))]` definitely disqualifies the enum from being ABI-compatible with T currently). I added the note about `packed` to match `align`, but `#[repr(packed)]` currently can't be applied to `enum`s at all anyway, so might be unnecessary. ----- I think this needs T-lang approval? cc ``````@workingjubilee``````
Improve clarity of `core::sync::atomic` docs about "Considerations" in regards to CAS operations ## Motivation The existing documentation for atomic `fetch_update` (and other similar methods) has a section that reads like so: > ### Considerations > This method is not magic; it is not provided by the hardware. It is implemented in > terms of `AtomicBlah::compare_exchange_weak`, and suffers from the same drawbacks. > In particular, this method will not circumvent the [ABA Problem]. > > [ABA Problem]: https://en.wikipedia.org/wiki/ABA_problem The wording here seems to imply that the drawbacks being discusses are caused by the *`weak` version* of `compare_exchange`, and that one may avoid those drawbacks by using `compare_exchange` instead. Indeed, a conversation in the `#dark-arts` channel on the Rust community discord based on this interpretation led to this PR. In reality, the drawbacks are inherent to implementing such an operation based on *any* compare-and-swap style operation, as opposed to an [LL,SC](https://en.wikipedia.org/wiki/Load-link/store-conditional) operation, and they apply equally to `compare_exchange` and `compare_exchange_weak` as well. ## Changes - Rewords existing Considerations section on `fetch_update` and friends to make clear that the limitations are inherent to an implementation based on any CAS operation, rather than the weak version of `compare_exchange` in particular. New version: > ### Considerations > > This method is not magic; it is not provided by the hardware, and does not act like a > critical section or mutex. > > It is implemented on top of an atomic [compare-and-swap operation], and thus is subject to > the usual drawbacks of CAS operations. In particular, be careful of the [ABA problem] > if this atomic integer is an index or more generally if knowledge of only the *bitwise value* > of the atomic is not in and of itself sufficient to ensure any required preconditions. > > [ABA Problem]: https://en.wikipedia.org/wiki/ABA_problem > [compare-and-swap operation]: https://en.wikipedia.org/wiki/Compare-and-swap - Add Considerations to `compare_exchange` and `compare_exchange_weak` which details similar considerations and when they may be relevant. New version: > ### Considerations > > `compare_exchange` is a [compare-and-swap operation] and thus exhibits the usual downsides > of CAS operations. In particular, a load of the value followed by a successful > `compare_exchange` with the previous load *does not ensure* that other threads have not > changed the value in the interim. This is usually important when the *equality* check in > the `compare_exchange` is being used to check the *identity* of a value, but equality > does not necessarily imply identity. In this case, `compare_exchange` can lead to the > [ABA problem]. > > [ABA Problem]: https://en.wikipedia.org/wiki/ABA_problem > [compare-and-swap operation]: https://en.wikipedia.org/wiki/Compare-and-swap
miri: add flag to suppress float non-determinism We have flags controlling most non-determinism, so this seems generally useful for debugging. It is also needed to work around rust-lang/portable-simd#463 in miri-test-libstd. I made this a rustc PR so that it propagates faster to unbreak miri-test-libstd. r? `@oli-obk`
The job Click to see the possible cause of the failure (guessed by this bot)
|
@bors retry |
@bors r+ rollup=never p=1 |
@bors p=6 |
@bors p=5 |
☀️ Test successful - checks-actions |
📌 Perf builds for each rolled up PR:
previous master: ed44c0e3b3 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing ed44c0e (parent) -> 015c777 (this PR) Test differencesShow 786 test diffsStage 1
Stage 2
Additionally, 782 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 015c7770ec0ffdba9ff03f1861144a827497f8ca --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (015c777): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -1.0%, secondary 1.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.6%, secondary 2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 756.165s -> 754.993s (-0.15%) |
Successful merges:
Clone
implementation forChunkBy
#138016 (AddedClone
implementation forChunkBy
)AttributeGate
andrustc_attr!
to emit notes during feature checking #141162 (refactorAttributeGate
andrustc_attr!
to emit notes during feature checking)ParseMode::Diagnostic
and fix multiline spans in diagnostic attribute lints #141474 (AddParseMode::Diagnostic
and fix multiline spans in diagnostic attribute lints)#[repr(Rust)]
to be ABI-compatible with their non-1ZST field. #141947 (Specify that "option-like" enums must be#[repr(Rust)]
to be ABI-compatible with their non-1ZST field.)core::sync::atomic
docs about "Considerations" in regards to CAS operations #142252 (Improve clarity ofcore::sync::atomic
docs about "Considerations" in regards to CAS operations)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup