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 #138151

Merged
merged 21 commits into from
Mar 7, 2025
Merged

Rollup of 8 pull requests #138151

merged 21 commits into from
Mar 7, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

pheki and others added 21 commits February 6, 2025 23:42
All other methods in this file have #[inline] and these methods are very
similar to those of &[u8] which are already inlined here.
When concatenating two WTF-8 strings, surrogate pairs at the boundaries
need to be joined. However, since UTF-8 strings cannot contain surrogate
halves, this check can be skipped when one string is UTF-8. Specialize
`OsString::push` to use a more efficient concatenation in this case.

Unfortunately, a specialization for `T: AsRef<str>` conflicts with
`T: AsRef<OsStr>`, so stamp out string types with a macro.
The WTF-8 version of `OsString` tracks whether it is known to be valid
UTF-8 with its `is_known_utf8` field. Specialize `From<AsRef<OsStr>>` so
this can be set for UTF-8 string types.
This patch makes BufReader::peek()'s doctest call read_more() to refill
the buffer before the inner reader hits EOF. This exposes a bug in
read_more() that causes an out-of-bounds slice access and segfault.
Buffer::read_more() is supposed to refill the buffer without discarding
its contents, which are in the range `pos .. filled`.

It mistakenly borrows the range `pos ..`, fills that, and then
increments `filled` by the amount read. This overwrites the buffer's
existing contents and sets `filled` to a too-large value that either
exposes uninitialized bytes or walks off the end of the buffer entirely.

This patch makes it correctly fill only the unfilled portion of the
buffer, which should maintain all the type invariants and fix the test
failure introduced in commit b119671.
…viper

Revert vita's c_char back to i8

# Description

Hi!

rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`.

I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.
…rsors, r=joboet

Override default `Write` methods for cursor-like types

Override the default `io::Write` methods for cursor-like types to provide more efficient versions.

Writes to resizable containers already write everything, so implement `write_all` and `write_all_vectored` in terms of those. For fixed-sized containers, cut out unnecessary error checking and looping for those same methods.

| `impl Write for T`              | `vectored` | `all` | `all_vectored` | `fmt`   |
| ------------------------------- | ---------- | ----- | -------------- | ------- |
| `&mut [u8]`                     | Y          | Y     | new            |         |
| `Vec<u8>`                       | Y          | Y     | new            | rust-lang#137762 |
| `VecDeque<u8>`                  | Y          | Y     | new            | rust-lang#137762 |
| `std::io::Cursor<&mut [u8]>`    | Y          | new   | new            |         |
| `std::io::Cursor<&mut Vec<u8>>` | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Vec<u8>>`      | Y          | new   | new            | rust-lang#137762 |
| `std::io::Cursor<Box<[u8]>>`    | Y          | new   | new            |         |
| `std::io::Cursor<[u8; N]>`      | Y          | new   | new            |         |
| `core::io::BorrowedCursor<'_>`  | new        | new   | new            |         |

Tracked in rust-lang#136756.

# Open questions

Is it guaranteed by `Write::write_all` that the maximal write is performed when not everything can be written? Its documentation describes the behavior of the default implementation, which writes until a 0-length write is encountered, thus implying that a maximal write is expected. In contrast, `Read::read_exact` declares that the contents of the buffer are unspecified for short reads. If it were allowed, these cursor-like types could bail on the write altogether if it has insufficient capacity.
…joboet

Specialize `OsString::push` and `OsString as From` for UTF-8

When concatenating two WTF-8 strings, surrogate pairs at the boundaries need to be joined. However, since UTF-8 strings cannot contain surrogate halves, this check can be skipped when one string is UTF-8. Specialize `OsString::push` to use a more efficient concatenation in this case.

The WTF-8 version of `OsString` tracks whether it is known to be valid UTF-8 with its `is_known_utf8` field. Specialize `From<AsRef<OsStr>>` so this can be set for UTF-8 string types.

Unfortunately, a specialization for `T: AsRef<str>` conflicts with `T: AsRef<OsStr>`, so stamp out string types with a macro.

r? ``@ChrisDenton``
Fix crash in BufReader::peek()

`bufreader_peek` tracking issue: rust-lang#128405

This fixes a logic error in `Buffer::read_more()` that would make `BufReader::peek()` expose uninitialized data and/or segfault if `read_more()` was called with a partially-full buffer and a non-empty inner reader.
…ilee

Improve the generic MIR in the default `PartialOrd::le` and friends

It looks like I regressed this accidentally in rust-lang#137197 due to rust-lang#137901

So this PR does two things:
1. Tweaks the way we're calling `is_some_and` so that it optimizes in the generic MIR (rather than needing to optimize it in every monomorphization) -- the first commit adds a MIR test, so you can see the difference in the second commit.
2. Updates the implementations of `is_le` and friends to be slightly simpler, and parallel how clang does them.
…yUwU

Suggest typo fix for static lifetime

...and don't try to introduce a new lifetime param named something like `'statoc`.
…gestion, r=compiler-errors

Simplify `printf` and shell format suggestions

Simplify tracking `printf` and shell format suggestions. Although allocations could be deferred until after checking that they aren't already in the map, this style is simpler.
…=tgross35

Stabilize const_char_classify, const_sockaddr_setters

FCP for const_char_classify: rust-lang#132241
FCP for const_sockaddr_setters: rust-lang#131714

Fixes rust-lang#132241
Fixes rust-lang#131714

Cc ``@rust-lang/wg-const-eval``
@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 Mar 7, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Mar 7, 2025

📌 Commit c33e9d6 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 Mar 7, 2025
@bors
Copy link
Contributor

bors commented Mar 7, 2025

⌛ Testing commit c33e9d6 with merge 59a9b9e9d776cb5d6bc02e99c4dce4f94f622232...

@bors
Copy link
Contributor

bors commented Mar 7, 2025

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 7, 2025
@bors bors merged commit 59a9b9e into rust-lang:master Mar 7, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 7, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#136667 Revert vita's c_char back to i8 7306982ca8e018658740391a8c98334ecfedc2d9 (link)
#137107 Override default Write methods for cursor-like types 6a17295d63a2e2d3df4ab419d0ea891c26b25080 (link)
#137777 Specialize OsString::push and OsString as From for UTF-8 26607c3a60714b3d3f69761e19f10f3aba294569 (link)
#137832 Fix crash in BufReader::peek() ea87248032e3fb665cfc64d0197a6dd61815c459 (link)
#137904 Improve the generic MIR in the default PartialOrd::le and… e87d9fc10d1a659f576dc1b70aa1301ec9246b8e (link)
#138115 Suggest typo fix for static lifetime 0a701c23cd328c810953d2d26d25cbaff03bcb00 (link)
#138125 Simplify printf and shell format suggestions 8cdaf7dd5f9b4e2b248405229441c4c94da49821 (link)
#138129 Stabilize const_char_classify, const_sockaddr_setters 2da9b8235419e382e197fcc96fc887a69498030d (link)

previous master: 91a0e1604f

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 (59a9b9e): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -0.9%, secondary -2.3%)

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)
5.7% [2.0%, 8.6%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.5% [-4.4%, -1.0%] 12
Improvements ✅
(secondary)
-2.3% [-4.3%, -0.6%] 39
All ❌✅ (primary) -0.9% [-4.4%, 8.6%] 15

Cycles

Results (primary -2.3%)

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
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Binary size

Results (primary -0.0%, secondary -0.1%)

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.0% [0.0%, 0.1%] 7
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.1%, -0.0%] 9
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.0%] 38
All ❌✅ (primary) -0.0% [-0.1%, 0.1%] 16

Bootstrap: 765.289s -> 766.319s (0.13%)
Artifact size: 362.07 MiB -> 362.07 MiB (0.00%)

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

10 participants