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 13 pull requests #101115

Merged
merged 32 commits into from
Aug 28, 2022
Merged

Rollup of 13 pull requests #101115

merged 32 commits into from
Aug 28, 2022

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

XrXr and others added 30 commits July 21, 2022 13:15
A colleague mentioned that they interpreted the old text
as saying that only the pointer and the length are copied.
Add a clause so it is more clear that the pointed to contents
are also copied.
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Aids the discoverability of `io::Error::last_os_error()` by linking to
commonly used error number functions from C/C++.
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
Signed-off-by: Nick Cameron <nrc@ncameron.org>
I wrote this code in 2017 back when std::fs::write wasn't available.
Also, use the t macro from tidy.
std::io: migrate ReadBuf to BorrowBuf/BorrowCursor

This PR replaces `ReadBuf` (used by the `Read::read_buf` family of methods) with `BorrowBuf` and `BorrowCursor`.

The general idea is to split `ReadBuf` because its API is large and confusing. `BorrowBuf` represents a borrowed buffer which is mostly read-only and (other than for construction) deals only with filled vs unfilled segments. a `BorrowCursor` is a mostly write-only view of the unfilled part of a `BorrowBuf` which distinguishes between initialized and uninitialized segments. For `Read::read_buf`, the caller would create a `BorrowBuf`, then pass a `BorrowCursor` to `read_buf`.

In addition to the major API split, I've made the following smaller changes:

* Removed some methods entirely from the API (mostly the functionality can be replicated with two calls rather than a single one)
* Unified naming, e.g., by replacing initialized with init and assume_init with set_init
* Added an easy way to get the number of bytes written to a cursor (`written` method)

As well as simplifying the API (IMO), this approach has the following advantages:

* Since we pass the cursor by value, we remove the 'unsoundness footgun' where a malicious `read_buf` could swap out the `ReadBuf`.
* Since `read_buf` cannot write into the filled part of the buffer, we prevent the filled part shrinking or changing which could cause underflow for the caller or unexpected behaviour.

## Outline

```rust
pub struct BorrowBuf<'a>

impl Debug for BorrowBuf<'_>

impl<'a> From<&'a mut [u8]> for BorrowBuf<'a>
impl<'a> From<&'a mut [MaybeUninit<u8>]> for BorrowBuf<'a>

impl<'a> BorrowBuf<'a> {
    pub fn capacity(&self) -> usize
    pub fn len(&self) -> usize
    pub fn init_len(&self) -> usize
    pub fn filled(&self) -> &[u8]
    pub fn unfilled<'this>(&'this mut self) -> BorrowCursor<'this, 'a>
    pub fn clear(&mut self) -> &mut Self
    pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
}

pub struct BorrowCursor<'buf, 'data>

impl<'buf, 'data> BorrowCursor<'buf, 'data> {
    pub fn clone<'this>(&'this mut self) -> BorrowCursor<'this, 'data>
    pub fn capacity(&self) -> usize
    pub fn written(&self) -> usize
    pub fn init_ref(&self) -> &[u8]
    pub fn init_mut(&mut self) -> &mut [u8]
    pub fn uninit_mut(&mut self) -> &mut [MaybeUninit<u8>]
    pub unsafe fn as_mut(&mut self) -> &mut [MaybeUninit<u8>]
    pub unsafe fn advance(&mut self, n: usize) -> &mut Self
    pub fn ensure_init(&mut self) -> &mut Self
    pub unsafe fn set_init(&mut self, n: usize) -> &mut Self
    pub fn append(&mut self, buf: &[u8])
}
```

## TODO

* ~~Migrate non-unix libs and tests~~
* ~~Naming~~
  * ~~`BorrowBuf` or `BorrowedBuf` or `SliceBuf`? (We might want an owned equivalent for the async IO traits)~~
  * ~~Should we rename the `readbuf` module? We might keep the name indicate it includes both the buf and cursor variations and someday the owned version too. Or we could change it. It is not publicly exposed, so it is not that important~~.
  * ~~`read_buf` method: we read into the cursor now, so the `_buf` suffix is a bit weird.~~
* ~~Documentation~~
* Tests are incomplete (I adjusted existing tests, but did not add new ones).

cc rust-lang#78485, rust-lang#94741
supersedes: rust-lang#95770, rust-lang#93359
fixes rust-lang#93305
…sleywiser

Add GDB/LLDB pretty-printers for NonZero types

Add GDB/LLDB pretty-printers for `NonZero` types.
These pretty-printers were originally implemented for IntelliJ Rust by ```@Kobzol``` in intellij-rust/intellij-rust#5270.

Part of rust-lang#29392.
Box::from(slice): Clarify that contents are copied

A colleague mentioned that they interpreted the old text
as saying that only the pointer and the length are copied.
Add a clause so it is more clear that the pointed to contents
are also copied.
…homcc

Add standard C error function aliases to last_os_error

This aids the discoverability of `io::Error::last_os_error()` by linking to commonly used error number functions from C/C++.

I've seen a few people not realize this exists, so hopefully this helps draw attention to the API to encourage using it over integer error codes.
Add mention of `BufReader` in `Read::bytes` docs

There is a general paragraph about `BufRead` in the `Read` trait's docs, however using `bytes` without `BufRead` *always* has a large impact, due to reads of size 1.

`@rustbot` label +A-docs
…, r=Mark-Simulacrum

Export Cancel from std::os::fortanix_sgx::usercalls::raw

This was missed in rust-lang#100642

cc ``@raoulstrackx`` and ``@jethrogb``
Some papercuts on error::Error

Renames the chain method, since I chain could mean anything and doesn't refer to a chain of sources (cc rust-lang#58520) (and adds a comment explaining why sources is not a provided method on Error). Renames arguments to the request method from `req` to `demand` since the type is `Demand` rather than Request or Requisition.

r? ``@yaahc``
Provide structured suggestion for `hashmap[idx] = val`
…i-obk

no alignment check during interning

This should fix rust-lang#101034
r? `@oli-obk`

Unfortunately we don't have a self-contained testcase for this problem. I am not sure how it can be triggered...
…er-errors

Use smaller span for suggestions
rustc_middle: Remove `Visibility::Invisible`

It had a different meaning in the past, but now it's only used as an implementation detail of import resolution.
…ark-Simulacrum

unstable-book-gen: use std::fs::write

I wrote this code in 2017 back when std::fs::write wasn't available.
Also, use the t macro from tidy.
@rustbot rustbot added 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 28, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=13

@bors
Copy link
Contributor

bors commented Aug 28, 2022

📌 Commit b3a4bc5 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 28, 2022
@bors
Copy link
Contributor

bors commented Aug 28, 2022

⌛ Testing commit b3a4bc5 with merge 3fdd578...

@bors
Copy link
Contributor

bors commented Aug 28, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 28, 2022
@bors bors merged commit 3fdd578 into rust-lang:master Aug 28, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 28, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3fdd578): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

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

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.6% [-0.6%, -0.6%] 1
Improvements ✅
(secondary)
-1.1% [-1.3%, -0.9%] 8
All ❌✅ (primary) -0.6% [-0.6%, -0.6%] 1

Max RSS (memory usage)

Results

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.

mean1 range count2
Regressions ❌
(primary)
1.7% [1.7%, 1.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.1% [-2.3%, -1.8%] 2
All ❌✅ (primary) 1.7% [1.7%, 1.7%] 1

Cycles

Results

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.

mean1 range count2
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.0% [-2.0%, -2.0%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-2.0%, 2.3%] 2

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@matthiaskrgr matthiaskrgr deleted the rollup-iy14ztr branch October 9, 2022 00:31
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.