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 7 pull requests #118433

Merged
merged 19 commits into from
Nov 29, 2023
Merged

Rollup of 7 pull requests #118433

merged 19 commits into from
Nov 29, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

joboet and others added 19 commits October 18, 2023 16:17
- Rename them both `as_str`, which is the typical name for a function
  that returns a `&str`. (`to_string` is appropriate for functions
  returning `String` or maybe `Cow<'a, str>`.)
- Change `UnOp::as_str` from an associated function (weird!) to a
  method.
- Avoid needless `self` dereferences.
To match `BinOpKind::is_comparison` and `hir::BinOpKind::is_lazy`.
They're identical to the same-named types from `ast`. I find it silly
(and inefficient) to have all this boilerplate code to convert one type
to an identical type.

There is already a small amount of type sharing between the AST and HIR,
e.g. `Attribute`, `MacroDef`.

The commit adds a `pub use` to `rustc_hir` so that, for example,
`ast::BinOp` can also be referred to as `hir::BinOp`. This is so the
many existing `hir`-qualified mentions of these types don't need to
change.

The commit also moves a couple of operations from the (removed) HIR
types to the AST types, e.g. `is_by_value`.
Validation introduced in rust-lang#113124 allows UnwindAction::Continue and
TerminatorKind::Resume to occur only in functions with ABI that can
unwind. The function ABI depends on the panic strategy, which can vary
across crates.

Usually MIR is built and validated in the same crate. The coroutine drop
glue thus far was an exception. As a result validation could fail when
mixing different panic strategies.

Avoid the problem by executing AbortUnwindingCalls along with the
validation.
Implement thread parking for xous

This follows the pattern set by [the Windows parker](https://github.com/rust-lang/rust/blob/ddef56d5dfa18f169af9db912dc8e8343797eebb/library/std/src/sys/windows/thread_parking.rs) when it uses keyed events. An atomic variable is used to track the state and optimize the fast path, while notifications are send via the ticktime server to block and unblock the thread.

ping `@xobs`
`@rustbot` label +T-libs +A-atomic
r? libs
remove the memcpy-on-equal-ptrs assumption

One of the libc we support, musl, [defines `memcpy` with `restrict` pointers](https://git.musl-libc.org/cgit/musl/tree/src/string/memcpy.c#n5). This in fact matches the definition in the C standard. Calling that `memcpy` with overlapping pointers is clearly UB, who knows what the compiler did when optimizing this `memcpy` -- it certainly assumed source and destination to be disjoint.

Lucky enough, it does not seem like we actually need this assumption that `memcpy(p, p, n)` is always allowed. clang and GCC need it since they use `memcpy` to compile C assignments, but [we use memmove for similar code](https://godbolt.org/z/bcW85WYcM). There are no known cases where LLVM introduces calls to memcpy on equal pointers itself. (And if there were, that would be a soundness bug in rustc due to the musl issue mentioned above.)

This does mean we must make sure to never call the LLVM `memcpy` builtin on equal ranges even though the LangRef says that is allowed. Currently that is the case so we just need to make sure it remains the case. :) Cc `@rust-lang/opsem` `@rust-lang/wg-llvm`
Unify `TraitRefs` and `PolyTraitRefs` in `ValuePairs`

I did this recently with `FnSigs` and `PolyFnSigs` but didn't think to do it with `TraitRefs` and `PolyTraitRefs`.
Remove HIR opkinds

`hir::BinOp`, `hir::BinOpKind`, and `hir::UnOp` are identical to `ast::BinOp`, `ast::BinOpKind`, and `ast::UnOp`, respectively. This seems silly, so this PR removes the HIR ones. (A re-export lets the AST ones be referred to using a `hir::` qualifier, which avoids renaming churn.)

r? `@cjgillot`
…illot

Eagerly return `ExprKind::Err` on `yield`/`await` in wrong coroutine context

This PR does 2 things:
1. Refuses to lower `.await` or `yield` when we are outside of the right coroutine context for the operator. Instead, we lower to `hir::ExprKind::Err`, to silence subsequent redundant errors.
2. Reworks a bit of the span tracking in `LoweringContext` to fix a bad span when we have something like `let x = [0; async_fn().await]` where the `await` is inside of an anon const. The span for the "item" still kinda sucks, since it overlaps with the `await` span, but at least it's accurate.
Fix coroutine validation for mixed panic strategy

Validation introduced in rust-lang#113124 allows `UnwindAction::Continue` and `TerminatorKind::Resume` to occur only in functions with ABI that can unwind. The function ABI depends on the panic strategy, which can vary across crates.

Usually MIR is built and validated in the same crate. The coroutine drop glue thus far was an exception. As a result validation could fail when mixing different panic strategies.

Avoid the problem by executing `AbortUnwindingCalls` along with the validation.

Fixes rust-lang#116953.
@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 Nov 29, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Nov 29, 2023

📌 Commit e8d0c56 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 Nov 29, 2023
@bors
Copy link
Contributor

bors commented Nov 29, 2023

⌛ Testing commit e8d0c56 with merge b06bbe7...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#116839 (Implement thread parking for xous)
 - rust-lang#118265 (remove the memcpy-on-equal-ptrs assumption)
 - rust-lang#118269 (Unify `TraitRefs` and `PolyTraitRefs` in `ValuePairs`)
 - rust-lang#118394 (Remove HIR opkinds)
 - rust-lang#118398 (Add proper cfgs in std)
 - rust-lang#118419 (Eagerly return `ExprKind::Err` on `yield`/`await` in wrong coroutine context)
 - rust-lang#118422 (Fix coroutine validation for mixed panic strategy)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Nov 29, 2023

💔 Test failed - checks-actions

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

@bors retry no logs

@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 Nov 29, 2023
@bors
Copy link
Contributor

bors commented Nov 29, 2023

⌛ Testing commit e8d0c56 with merge ec1f21c...

@bors
Copy link
Contributor

bors commented Nov 29, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 29, 2023
@bors bors merged commit ec1f21c into rust-lang:master Nov 29, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 29, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#116839 Implement thread parking for xous 07b03a2bb507f8385b0ce827de98516673f04de8 (link)
#118265 remove the memcpy-on-equal-ptrs assumption 0d787e46f42564b22df37c03bca9a7b6d2359c85 (link)
#118269 Unify TraitRefs and PolyTraitRefs in ValuePairs 40a94c930b8f048f1f9450e2f1d0a0b6c1530c38 (link)
#118394 Remove HIR opkinds e9f058fd63f2cc70d05a02a2413cde91a664ce5b (link)
#118398 Add proper cfgs in std 350d52e0d971f698034faacaf41a707f03c4b2fd (link)
#118419 Eagerly return ExprKind::Err on yield/await in wrong … c83430d154b7299415ee60f015c2381f1863515b (link)
#118422 Fix coroutine validation for mixed panic strategy 3eae123b2e16c91ac17d8b268e4fc2834d41fd78 (link)

previous master: e9b7bf0114

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 (ec1f21c): 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.

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

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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.8% [-4.4%, -1.2%] 3
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 673.972s -> 673.349s (-0.09%)
Artifact size: 313.37 MiB -> 313.36 MiB (-0.00%)

@matthiaskrgr matthiaskrgr deleted the rollup-fi9lrwg branch March 16, 2024 18:19
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.