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 6 pull requests #130847

Merged
merged 13 commits into from
Sep 25, 2024
Merged

Rollup of 6 pull requests #130847

merged 13 commits into from
Sep 25, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

monkeydbobo and others added 13 commits September 24, 2024 19:51
…=spastorino,RalfJung

Simple validation for unsize coercion in MIR validation

This adds the most basic validity check to unsize coercions in MIR. The src and target of an unsize cast must *at least* implement `Src: CoerceUnsized<Target>` for this to be valid.

This doesn't the second, more subtle validity check that is taken of advantage in codegen [here](https://github.com/rust-lang/rust/blob/914193c8f40528fe82696e1054828de8c399882e/compiler/rustc_codegen_ssa/src/base.rs#L126), but I did leave a beefy FIXME for that explaining what it is.

As a consequence, this also fixes an ICE with GVN and invalid unsize coercions. This is somewhat coincidental, since MIR inlining will check that a body is valid before inlining it; so now that we determine it to be invalid, we don't inline it, and we don't encounter the GVN ICE. I'm not certain if the same GVN ICE is triggerable without the inliner, and perhaps instead with trivial where clauses or something.

cc `@RalfJung`
…le_osx, r=davidtwco

Fix up setting strip = true in Cargo.toml makes build scripts fail in…

Fix issue: rust-lang#110536
Strip binary is PATH dependent which breaks builds in MacOS.
For example, on my Mac, the output of 'which strip' is '/opt/homebrew/opt/binutils/bin/strip', which leads to incorrect 'strip' results. Therefore, just like on other systems, it is also necessary to specify 'stripcmd' on macOS. However, it seems that there is a bug in binutils [bugzilla-Bug 31571](https://sourceware.org/bugzilla/show_bug.cgi?id=31571), which leads to the problem mentioned above.
add link from random() helper fn to extensive DefaultRandomSource docs
…r=Noratrieb

Add `must_use` attribute to `len_utf8` and `len_utf16`.

The `len_utf8` and `len_utf16` methods in `char` should have the `must_use` attribute.

The somewhat similar method `<[T]>::len` has had this attribute since rust-lang#95274. Considering that these two methods would most likely be used to test the size of a buffer (before a call to `encode_utf8` or `encode_utf16`), *not* using their return values could indicate a bug.

According to ["When to add `#[must_use]`](https://std-dev-guide.rust-lang.org/policy/must-use.html), this is **not** considered a breaking change (and could be reverted again at a later time).
…ubilee

fix some cfg logic around optimize_for_size and 16-bit targets

Fixes rust-lang#130818.
Fixes rust-lang#129910.

There are still some warnings when building on a 16bit target:
```
warning: struct `AlignedStorage` is never constructed
   --> /home/r/src/rust/rustc.2/library/core/src/slice/sort/stable/mod.rs:135:8
    |
135 | struct AlignedStorage<T, const N: usize> {
    |        ^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

warning: associated items `new` and `as_uninit_slice_mut` are never used
   --> /home/r/src/rust/rustc.2/library/core/src/slice/sort/stable/mod.rs:141:8
    |
140 | impl<T, const N: usize> AlignedStorage<T, N> {
    | -------------------------------------------- associated items in this implementation
141 |     fn new() -> Self {
    |        ^^^
...
145 |     fn as_uninit_slice_mut(&mut self) -> &mut [MaybeUninit<T>] {
    |        ^^^^^^^^^^^^^^^^^^^

warning: function `quicksort` is never used
  --> /home/r/src/rust/rustc.2/library/core/src/slice/sort/unstable/quicksort.rs:19:15
   |
19 | pub(crate) fn quicksort<'a, T, F>(
   |               ^^^^^^^^^

warning: `core` (lib) generated 3 warnings
```

However, the cfg stuff here is sufficiently messy that I didn't want to touch more of it. I think all `feature = "optimize_for_size"` should become `any(feature = "optimize_for_size", target_pointer_width = "16")` but I am not entirely certain. Warnings are fine, Miri will just ignore them.

Cc `@Voultapher`
…s, r=jieyouxu

Add tracking issue for io_error_inprogress

I forgot to mention this in rust-lang#130789
@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 Sep 25, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=6

@bors
Copy link
Contributor

bors commented Sep 25, 2024

📌 Commit e805182 has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Sep 25, 2024
@bors
Copy link
Contributor

bors commented Sep 25, 2024

⌛ Testing commit e805182 with merge 0399709...

@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #130847!

Tested on commit 0399709.
Direct link to PR: #130847

💔 reference on windows: test-pass → test-fail (cc @ehuss @Havvy @matthewjasper).
💔 reference on linux: test-pass → test-fail (cc @ehuss @Havvy @matthewjasper).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Sep 25, 2024
Tested on commit rust-lang/rust@0399709.
Direct link to PR: <rust-lang/rust#130847>

💔 reference on windows: test-pass → test-fail (cc @ehuss @Havvy @matthewjasper).
💔 reference on linux: test-pass → test-fail (cc @ehuss @Havvy @matthewjasper).
@bors
Copy link
Contributor

bors commented Sep 25, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2024
@bors bors merged commit 0399709 into rust-lang:master Sep 25, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 25, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#130735 Simple validation for unsize coercion in MIR validation 323f521bc6d8f2b966ba7838a3f3ee364e760b7e (link)
#130781 Fix up setting strip = true in Cargo.toml makes build scrip… 07f6a1a0132cea6cdbb792c0088b2b431f93d8ee (link)
#130811 add link from random() helper fn to extensive DefaultRandom… bf043b2b40c2bf079f60502f9fe165f1741ba0c1 (link)
#130819 Add must_use attribute to len_utf8 and len_utf16. 5e368e2e4ba8d0d61f35259dd2cf00394073d2d5 (link)
#130832 fix some cfg logic around optimize_for_size and 16-bit targ… 66cf6e658fac04970c9fdafdbab86baebdb88ccb (link)
#130842 Add tracking issue for io_error_inprogress c8038d4122603a53b36ca6cbe60109b113da6a92 (link)

previous master: b5117538e9

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

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0399709): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.3%, 0.9%] 9
Regressions ❌
(secondary)
0.6% [0.3%, 1.8%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.6% [0.3%, 0.9%] 9

Max RSS (memory usage)

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

Cycles

Results (primary 0.7%, secondary 2.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)
2.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

Binary size

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

Bootstrap: 769.756s -> 772.322s (0.33%)
Artifact size: 340.86 MiB -> 340.90 MiB (0.01%)

@Kobzol
Copy link
Contributor

Kobzol commented Oct 1, 2024

Regression comes from #130735, which implements new functionality (new form of MIR validation).

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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.