Skip to content

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

Merged
merged 22 commits into from
Jun 13, 2025
Merged

Rollup of 6 pull requests #142432

merged 22 commits into from
Jun 13, 2025

Conversation

matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Jun 12, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

nwoods-cimpress and others added 22 commits March 10, 2025 11:12
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`
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jun 12, 2025
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Executing TIDY_PRINT_DIFF=1 npm install eslint@$(head -n 1 /tmp/eslint.version) &&  python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
+ head -n 1 /tmp/eslint.version
+ TIDY_PRINT_DIFF=1 npm install [email protected]
npm ERR! code E502
npm ERR! 502 Bad Gateway - GET https://registry.npmjs.org/@humanwhocodes%2fobject-schema

npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-06-12T18_06_57_099Z-debug-0.log
  local time: Thu Jun 12 18:11:31 UTC 2025
  network time: Thu, 12 Jun 2025 18:11:31 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@workingjubilee workingjubilee added the CI-spurious-fail-npm we forgot to cache our installs from npm label Jun 12, 2025
@matthiaskrgr
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=1

@bors
Copy link
Collaborator

bors commented Jun 12, 2025

📌 Commit 88df5a5 has been approved by matthiaskrgr

It is now in the queue for this repository.

@matthiaskrgr
Copy link
Member Author

@bors p=6

@matthiaskrgr
Copy link
Member Author

@bors p=5

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

⌛ Testing commit 88df5a5 with merge 015c777...

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 015c777 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 13, 2025
@bors bors merged commit 015c777 into rust-lang:master Jun 13, 2025
17 of 20 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 13, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#138016 Added Clone implementation for ChunkBy bb7cd13cdd317fb51f69d4a43b310a31ad7c0e3d (link)
#141162 refactor AttributeGate and rustc_attr! to emit notes d… bf00daa0f21c7c3f6d534f556305e4fe8e33e25e (link)
#141474 Add ParseMode::Diagnostic and fix multiline spans in diag… 67a65dce5a0d66fa3577e0d74544aee8c554d7be (link)
#141947 Specify that "option-like" enums must be #[repr(Rust)] to… db2ec96a08adc59b0aad0d87583e7d65ee9b6c92 (link)
#142252 Improve clarity of core::sync::atomic docs about "Conside… 2d06cff21f87b0cf34cf4b1e5766864c544090bc (link)
#142337 miri: add flag to suppress float non-determinism 59a24aab6b4514dd0952657ba37b54c4f75c787b (link)

previous master: ed44c0e3b3

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

Copy link
Contributor

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 differences

Show 786 test diffs

Stage 1

  • tests::diagnostic_format_flags: [missing] -> pass (J1)
  • tests::diagnostic_format_mod: [missing] -> pass (J1)
  • [ui] tests/ui/diagnostic_namespace/multiline_spans.rs: [missing] -> pass (J2)

Stage 2

  • [ui] tests/ui/diagnostic_namespace/multiline_spans.rs: [missing] -> pass (J0)

Additionally, 782 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 015c7770ec0ffdba9ff03f1861144a827497f8ca --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-2: 4793.3s -> 5850.1s (22.0%)
  2. aarch64-apple: 5019.4s -> 3985.7s (-20.6%)
  3. x86_64-apple-1: 6424.0s -> 7441.9s (15.8%)
  4. x86_64-msvc-ext2: 5460.9s -> 6245.0s (14.4%)
  5. dist-various-1: 4191.7s -> 4769.1s (13.8%)
  6. dist-apple-various: 6953.1s -> 6005.2s (-13.6%)
  7. i686-gnu-nopt-1: 8001.6s -> 9018.8s (12.7%)
  8. mingw-check-tidy: 74.9s -> 66.9s (-10.7%)
  9. dist-sparcv9-solaris: 5451.9s -> 4905.7s (-10.0%)
  10. dist-x86_64-apple: 8564.2s -> 7791.1s (-9.0%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (015c777): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This 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.

mean range count
Regressions ❌
(primary)
1.0% [1.0%, 1.0%] 1
Regressions ❌
(secondary)
4.1% [2.1%, 6.2%] 2
Improvements ✅
(primary)
-3.0% [-3.0%, -3.0%] 1
Improvements ✅
(secondary)
-3.0% [-3.0%, -3.0%] 1
All ❌✅ (primary) -1.0% [-3.0%, 1.0%] 2

Cycles

Results (primary -2.6%, secondary 2.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.5%, 3.2%] 2
Improvements ✅
(primary)
-2.6% [-3.3%, -1.0%] 6
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.6% [-3.3%, -1.0%] 6

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 756.165s -> 754.993s (-0.15%)
Artifact size: 372.25 MiB -> 372.27 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-spurious-fail-npm we forgot to cache our installs from npm merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.