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 5 pull requests #125313

Merged
merged 17 commits into from
May 20, 2024
Merged

Rollup of 5 pull requests #125313

merged 17 commits into from
May 20, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

zachs18 and others added 17 commits May 13, 2024 17:49
... since fn allocator doesn't exist yet.
(libs team decided not to add `absurd` to std)
     Locking 24 packages to latest compatible versions
    Updating ammonia v3.3.0 -> v4.0.0
    Updating anyhow v1.0.83 -> v1.0.86
    Updating camino v1.1.6 -> v1.1.7
    Updating darling v0.20.8 -> v0.20.9
    Updating darling_core v0.20.8 -> v0.20.9
    Updating darling_macro v0.20.8 -> v0.20.9
      Adding dbus v0.9.7
    Updating either v1.11.0 -> v1.12.0
      Adding html5ever v0.27.0
    Updating instant v0.1.12 -> v0.1.13
      Adding libdbus-sys v0.2.5
    Updating linux-raw-sys v0.4.13 -> v0.4.14 (latest: v0.6.4)
      Adding markup5ever v0.12.1
    Updating mdbook v0.4.37 -> v0.4.40
    Updating miniz_oxide v0.7.2 -> v0.7.3
      Adding opener v0.7.1
    Updating rustversion v1.0.16 -> v1.0.17
    Updating serde v1.0.201 -> v1.0.202
    Updating serde_derive v1.0.201 -> v1.0.202
    Updating serde_spanned v0.6.5 -> v0.6.6
    Removing strsim v0.10.0
    Updating syn v2.0.62 -> v2.0.64
    Updating thiserror v1.0.60 -> v1.0.61
    Updating thiserror-impl v1.0.60 -> v1.0.61
    Updating toml_datetime v0.6.5 -> v0.6.6
note: pass `--verbose` to see 96 unchanged dependencies behind latest
Coroutines can be prefixed with the `static` keyword to make them
`!Unpin`.
However, given the following function:

```rust

fn check() -> impl Sized {
    let x = 0;
    #[coroutine]
    static || {
        yield;
        x
    }
}
```

We currently suggest prefixing `move` before `static`, which is
syntactically incorrect:

```
error[E0373]: coroutine may outlive the current function, but it borrows
...
 --> src/main.rs:6:5
  |
6 |     static || {
  |     ^^^^^^^^^ may outlive borrowed value `x`
7 |         yield;
8 |         x
  |         - `x` is borrowed here
  |
note: coroutine is returned here
 --> src/main.rs:6:5
  |
6 | /     static || {
7 | |         yield;
8 | |         x
9 | |     }
  | |_____^
help: to force the coroutine to take ownership of `x` (and any other
referenced variables), use the `move` keyword
  |     // this is syntactically incorrect, it should be `static move ||`
6 |     move static || {
  |     ++++

```

This PR suggests adding `move` after `static` for these coroutines.
An earlier commit included the change for a suggestion here.
Unfortunately, it also used unwrap instead of dying properly.
Roll out the ~~rice paper~~ EarlyDiagCtxt before we do anything that
might leave a mess.
…ulacrum

Weekly `cargo update`

Automation to keep dependencies in `Cargo.lock` current.

The following is the output from `cargo update`:

```txt
     Locking 24 packages to latest compatible versions
    Updating ammonia v3.3.0 -> v4.0.0
    Updating anyhow v1.0.83 -> v1.0.86
    Updating camino v1.1.6 -> v1.1.7
    Updating darling v0.20.8 -> v0.20.9
    Updating darling_core v0.20.8 -> v0.20.9
    Updating darling_macro v0.20.8 -> v0.20.9
      Adding dbus v0.9.7
    Updating either v1.11.0 -> v1.12.0
      Adding html5ever v0.27.0
    Updating instant v0.1.12 -> v0.1.13
      Adding libdbus-sys v0.2.5
    Updating linux-raw-sys v0.4.13 -> v0.4.14 (latest: v0.6.4)
      Adding markup5ever v0.12.1
    Updating mdbook v0.4.37 -> v0.4.40
    Updating miniz_oxide v0.7.2 -> v0.7.3
      Adding opener v0.7.1
    Updating rustversion v1.0.16 -> v1.0.17
    Updating serde v1.0.201 -> v1.0.202
    Updating serde_derive v1.0.201 -> v1.0.202
    Updating serde_spanned v0.6.5 -> v0.6.6
    Removing strsim v0.10.0
    Updating syn v2.0.62 -> v2.0.64
    Updating thiserror v1.0.60 -> v1.0.61
    Updating thiserror-impl v1.0.60 -> v1.0.61
    Updating toml_datetime v0.6.5 -> v0.6.6
note: pass `--verbose` to see 96 unchanged dependencies behind latest
```
…-only, r=Mark-Simulacrum

Add `fn into_raw_with_allocator` to Rc/Arc/Weak.

Split out from rust-lang#119761

Add `fn into_raw_with_allocator` for `Rc`/`rc::Weak`[^1]/`Arc`/`sync::Weak`.
* Pairs with `from_raw_in` (which already exists on all 4 types).
* Name matches `Box::into_raw_with_allocator`.
* Associated fns on `Rc`/`Arc`, methods on `Weak`s.

<details> <summary>Future PR/ACP</summary>

As a follow-on to this PR, I plan to make a PR/ACP later to move `into_raw(_parts)` from `Container<_, A: Allocator>` to only `Container<_, Global>` (where `Container` = `Vec`/`Box`/`Rc`/`rc::Weak`/`Arc`/`sync::Weak`) so that users of non-`Global` allocators have to explicitly handle the allocator when using `into_raw`-like APIs.

The current behaviors of stdlib containers are inconsistent with respect to what happens to the allocator when `into_raw` is called (which does not return the allocator)

| Type | `into_raw` currently callable with | behavior of `into_raw`|
| --- | --- | --- |
| `Box` | any allocator | allocator is [dropped](https://doc.rust-lang.org/nightly/src/alloc/boxed.rs.html#1060) |
| `Vec` | any allocator | allocator is [forgotten](https://doc.rust-lang.org/nightly/src/alloc/vec/mod.rs.html#884) |
| `Arc`/`Rc`/`Weak` | any allocator | allocator is [forgotten](https://doc.rust-lang.org/src/alloc/sync.rs.html#1487)(Arc) [(sync::Weak)](https://doc.rust-lang.org/src/alloc/sync.rs.html#2726) [(Rc)](https://doc.rust-lang.org/src/alloc/rc.rs.html#1352) [(rc::Weak)](https://doc.rust-lang.org/src/alloc/rc.rs.html#2993) |

In my opinion, neither implicitly dropping nor implicitly forgetting the allocator is ideal; dropping it could immediately invalidate the returned pointer, and forgetting it could unintentionally leak memory. My (to-be) proposed solution is to just forbid calling `into_raw(_parts)` on containers with non-`Global` allocators, and require calling `into_raw_with_allocator`(/`Vec::into_raw_parts_with_alloc`)

</details>

[^1]:  Technically, `rc::Weak::into_raw_with_allocator` is not newly added, as it was modified and renamed from `rc::Weak::into_raw_and_alloc`.
…rovements, r=compiler-errors

Never type unsafe lint improvements

- Move linting code to a separate method
- Remove mentions of `core::convert::absurd` (rust-lang#124311 was rejected)
- Make the lint into FCW

The last thing is a bit weird though. On one hand it should be `EditionSemanticsChange(2024)`, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. _Also_, it's weird that we don't have `FutureReleaseSemanticsChangeReportInDeps`, IMO "this might cause UB in a future release" is important enough to be reported in deps...

IMO we ought to have three enums instead of [`FutureIncompatibilityReason`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.FutureIncompatibilityReason.html#):

```rust
enum IncompatibilityWhen {
     FutureRelease,
     Edition(Edition),
}

enum IncompatibilyWhat {
    Error,
    SemanticChange,
}

enum IncompatibilityReportInDeps {
    No,
    Yes,
}
```

Tracking:
- rust-lang#123748
…r=compiler-errors

fix suggestion in E0373 for !Unpin coroutines

Coroutines can be prefixed with the `static` keyword to make them
`!Unpin`.
However, given the following function:

```rust

fn check() -> impl Sized {
    let x = 0;
    #[coroutine]
    static || {
        yield;
        x
    }
}
```

We currently suggest prefixing `move` before `static`, which is
syntactically incorrect:

```
error[E0373]: coroutine may outlive the current function, but it borrows
...
 --> src/main.rs:6:5
  |
6 |     static || {
  |     ^^^^^^^^^ may outlive borrowed value `x`
7 |         yield;
8 |         x
  |         - `x` is borrowed here
  |
note: coroutine is returned here
 --> src/main.rs:6:5
  |
6 | /     static || {
7 | |         yield;
8 | |         x
9 | |     }
  | |_____^
help: to force the coroutine to take ownership of `x` (and any other
referenced variables), use the `move` keyword
  |     // this is syntactically incorrect, it should be `static move ||`
6 |     move static || {
  |     ++++

```

This PR suggests adding `move` after `static` for these coroutines.

I also added a UI test for this case.
…t, r=compiler-errors

defrost `RUST_MIN_STACK=ice rustc hello.rs`

I didn't think too hard about testing my previous PR rust-lang#122847 which makes our stack overflow handler assist people in discovering the `RUST_MIN_STACK` variable (which apparently is surprisingly useful for Really Big codebases). After it was merged, some useful comments left in a drive-by review led me to discover I had added an ICE. This reworks the code a bit to explain the rationale, remove the ICE that I introduced, and properly test one of the diagnostics.
@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 May 20, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented May 20, 2024

📌 Commit ecbd110 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 May 20, 2024
@bors
Copy link
Contributor

bors commented May 20, 2024

⌛ Testing commit ecbd110 with merge e8ada6a...

@bors
Copy link
Contributor

bors commented May 20, 2024

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

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#125034 Weekly cargo update 8076a238ad885f2a9439ceaf2d584aaa81ca0d7f (link)
#125093 Add fn into_raw_with_allocator to Rc/Arc/Weak. 6c49d0d8055195915fb06de813a0be1c94d72cd1 (link)
#125282 Never type unsafe lint improvements 09a7b28b5b7d458b4e12e2bc36bac50b50d0ee79 (link)
#125301 fix suggestion in E0373 for !Unpin coroutines 1ebf33c4ba3d9b3477523640ed4be187414354f2 (link)
#125302 defrost RUST_MIN_STACK=ice rustc hello.rs 05c46f018c51407c29cdba266c840a207d0ff478 (link)

previous master: f092f73c11

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 (e8ada6a): 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 2.2%)

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

Cycles

Results (secondary -0.6%)

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)
3.8% [2.0%, 5.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.8% [-3.2%, -2.4%] 6
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.4%, secondary -1.0%)

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)
-0.4% [-1.0%, -0.1%] 15
Improvements ✅
(secondary)
-1.0% [-1.4%, -0.2%] 39
All ❌✅ (primary) -0.4% [-1.0%, -0.1%] 15

Bootstrap: 671.026s -> 670.321s (-0.11%)
Artifact size: 316.08 MiB -> 316.18 MiB (0.03%)

@matthiaskrgr matthiaskrgr deleted the rollup-65etxv0 branch September 1, 2024 17:35
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.

8 participants