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

Merged
merged 25 commits into from
Jan 24, 2025
Merged

Rollup of 7 pull requests #135947

merged 25 commits into from
Jan 24, 2025

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

joshtriplett and others added 25 commits January 11, 2025 06:35
Approved ACP: rust-lang/libs-team#502
Tracking issue: rust-lang#134915

These types represent human-readable strings that are conventionally,
but not always, UTF-8. The `Debug` impl prints non-UTF-8 bytes using
escape sequences, and the `Display` impl uses the Unicode replacement
character.

This is a minimal implementation of these types and associated trait
impls. It does not add any helper methods to other types such as `[u8]`
or `Vec<u8>`.

I've omitted a few implementations of `AsRef`, `AsMut`, `Borrow`,
`From`, and `PartialOrd`, when those would be the second implementation
for a type (counting the `T` impl) or otherwise may cause inference
failures. These impls are important, but we can attempt to add them
later in standalone commits, and run them through crater.

In addition to the `bstr` feature, I've added a `bstr_internals` feature
for APIs provided by `core` for use by `alloc` but not currently
intended for stabilization.

This API and its implementation are based *heavily* on the `bstr` crate
by Andrew Gallant (@BurntSushi).
The Deref brings in the documentation from slice, so it has the same
issue as slice.
For now, apply `no_global_oom_handling` to all of
library/alloc/src/bstr.rs . We can make it more fine-grained later.
Remove comment about missing integrated assembler handling, which was
removed in commit 02840ca.
They can both be set inside the config callback too.
It has become nothing other than a wrapper around run_compiler.
Implement `ByteStr` and `ByteString` types

Approved ACP: rust-lang/libs-team#502
Tracking issue: rust-lang#134915

These types represent human-readable strings that are conventionally,
but not always, UTF-8. The `Debug` impl prints non-UTF-8 bytes using
escape sequences, and the `Display` impl uses the Unicode replacement
character.

This is a minimal implementation of these types and associated trait
impls. It does not add any helper methods to other types such as `[u8]`
or `Vec<u8>`.

I've omitted a few implementations of `AsRef`, `AsMut`, and `Borrow`,
when those would be the second implementation for a type (counting the
`T` impl), to avoid potential inference failures. We can attempt to add
more impls later in standalone commits, and run them through crater.

In addition to the `bstr` feature, I've added a `bstr_internals` feature
for APIs provided by `core` for use by `alloc` but not currently
intended for stabilization.

This API and its implementation are based *heavily* on the `bstr` crate
by Andrew Gallant (`@BurntSushi).`

r? `@BurntSushi`
…st, r=compiler-errors

Add missing check for async body when suggesting await on futures.

Currently the compiler suggests adding `.await` to resolve some type conflicts without checking if the conflict happens in an async context. This can lead to the compiler suggesting `.await` in function signatures where it is invalid. Example:

```rs
trait A {
    fn a() -> impl Future<Output = ()>;
}
struct B;
impl A for B {
    fn a() -> impl Future<Output = impl Future<Output = ()>> {
        async { async { () } }
    }
}
```
```
error[E0271]: expected `impl Future<Output = impl Future<Output = ()>>` to be a future that resolves to `()`, but it resolves to `impl Future<Output = ()>`
 --> bug.rs:6:15
  |
6 |     fn a() -> impl Future<Output = impl Future<Output = ()>> {
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found future
  |
note: calling an async function returns a future
 --> bug.rs:6:15
  |
6 |     fn a() -> impl Future<Output = impl Future<Output = ()>> {
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required by a bound in `A::{synthetic#0}`
 --> bug.rs:2:27
  |
2 |     fn a() -> impl Future<Output = ()>;
  |                           ^^^^^^^^^^^ required by this bound in `A::{synthetic#0}`
help: consider `await`ing on the `Future`
  |
6 |     fn a() -> impl Future<Output = impl Future<Output = ()>>.await {
  |                                                             ++++++
```

The documentation of suggest_await_on_expect_found (`compiler/rustc_trait_selection/src/error_reporting/infer/suggest.rs:156`) even mentions such a check but does not actually implement it.

This PR adds that check to ensure `.await` is only suggested within async blocks.

There were 3 unit tests whose expected output needed to be changed because they had the suggestion outside of async. One of them (`tests/ui/async-await/dont-suggest-missing-await.rs`) actually tests that exact problem but expects it to be present.

Thanks to `@llenck` for initially noticing the bug and helping with fixing it
…ler-errors

handle global trait bounds defining assoc types

This also fixes the compare-mode for
- tests/ui/coherence/coherent-due-to-fulfill.rs
- tests/ui/codegen/mono-impossible-2.rs
- tests/ui/trivial-bounds/trivial-bounds-inconsistent-projection.rs
- tests/ui/nll/issue-61320-normalize.rs

I first considered the alternative to always prefer where-bounds during normalization, regardless of how the trait goal has been proven by changing `fn merge_candidates` instead. https://github.com/rust-lang/rust/blob/ecda83b30f0f68cf5692855dddc0bc38ee8863fc/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs#L785

This approach is more restrictive than behavior of the old solver to avoid mismatches between trait and normalization goals. This may be breaking in case the where-bound adds unnecessary region constraints and we currently don't ever try to normalize an associated type. I would like to detect these cases and change the approach to exactly match the old solver if required. I want to minimize cases where attempting to normalize in more places causes code to break.

r? `@compiler-errors`
…i-obk

Get rid of RunCompiler

The various `set_*` methods that have been removed can be replaced by setting the respective fields in the `Callbacks::config` implementation. `set_using_internal_features` was often forgotten and it's equivalent is now done automatically.
…nt, r=compiler-errors

rustc_codegen_llvm: remove outdated asm-to-obj codegen note

Remove comment about missing integrated assembler handling, which was removed in commit 02840ca.
…piler-errors

Allow `arena_cache` queries to return `Option<&'tcx T>`

Currently, `arena_cache` queries always have to return `&'tcx T`[^deref]. This means that if an arena-cached query wants to return an optional value, it has to return `&'tcx Option<T>`, which has a few negative consequences:

- It goes against normal Rust style, where `Option<&T>` is preferred over `&Option<T>`.
- Callers that actually want an `Option<&T>` have to manually call `.as_ref()` on the query result.
- When the query result is `None`, a full-sized `Option<T>` still needs to be stored in the arena.

This PR solves that problem by introducing a helper trait `ArenaCached` that is implemented for both `&T` and `Option<&T>`, and takes care of bridging between the provided type, the arena-allocated type, and the declared query return type.

---

To demonstrate that this works, I have converted the two existing arena-cached queries that currently return `&Option<T>`: `mir_coroutine_witnesses` and `diagnostic_hir_wf_check`. Only the query declarations need to be modified; existing providers and callers continue to work with the new query return type.

(My real goal is to apply this to `coverage_ids_info`, which will return Option as of rust-lang#135873, but that PR hasn't landed yet.)

[^deref]: Technically they could return other types that implement `Deref`, but it's hard to imagine this working well with anything other than `&T`.
simplify parse_format::Parser::ws by using next_if
@rustbot
Copy link
Collaborator

rustbot commented Jan 23, 2025

Could not assign reviewer from: ghost.
User(s) ghost are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 23, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2025
@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jan 23, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Jan 23, 2025

📌 Commit 7d26006 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 Jan 23, 2025
@bors
Copy link
Contributor

bors commented Jan 24, 2025

⌛ Testing commit 7d26006 with merge 1c9837d...

@bors
Copy link
Contributor

bors commented Jan 24, 2025

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

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1c9837d): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

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.7% [0.1%, 2.2%] 15
Regressions ❌
(secondary)
1.4% [0.1%, 2.2%] 23
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.7% [0.1%, 2.2%] 15

Max RSS (memory usage)

Results (primary 1.0%, secondary -0.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)
1.8% [0.8%, 3.4%] 3
Regressions ❌
(secondary)
1.0% [1.0%, 1.0%] 3
Improvements ✅
(primary)
-1.3% [-1.3%, -1.3%] 1
Improvements ✅
(secondary)
-2.4% [-2.8%, -1.9%] 2
All ❌✅ (primary) 1.0% [-1.3%, 3.4%] 4

Cycles

Results (primary 4.2%, secondary 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)
4.2% [1.1%, 8.5%] 15
Regressions ❌
(secondary)
2.2% [1.7%, 2.5%] 10
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 4.2% [1.1%, 8.5%] 15

Binary size

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

Bootstrap: 774.487s -> 775.826s (0.17%)
Artifact size: 326.07 MiB -> 325.94 MiB (-0.04%)

@rustbot rustbot added the perf-regression Performance regression. label Jan 24, 2025
@matthiaskrgr
Copy link
Member Author

135880 changed rustdoc

@matthiaskrgr
Copy link
Member Author

shouldnt there be a rust timer list coment to run the command on the single prs? 🤔

@Kobzol
Copy link
Contributor

Kobzol commented Jan 24, 2025

Yeah, there should be. Maybe GH or the perfbot was temporarily offline, on the next rollup it worked (#135959 (comment)).

@matthiaskrgr matthiaskrgr deleted the rollup-k9jpfls branch January 25, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustc-dev-guide Area: rustc-dev-guide merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.