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

Merged
merged 18 commits into from
Aug 30, 2023
Merged

Rollup of 7 pull requests #115370

merged 18 commits into from
Aug 30, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

workingjubilee and others added 18 commits July 19, 2023 00:15
...to both compiler contributors and other rustc invokers.
The previous error messaging was extremely unfriendly,
and potentially both confusing and alarming.

The entire modules is extracted into a new file to help
ease of reading, and plenty of comments get layered in.
The design of the messaging is focused on preventing
the overall purpose of the output from being too opaque
in common cases, so users understand why it is there.
There's not an immediate need for it being actionable,
but some suggestions are offered anyways.
This cleans up the formatting of the backtrace considerably,
dodging the fact that a backtrace with recursion normally
emits hundreds of lines. Instead, simply write repeated symbols,
and add how many times the recursion occurred.
Also, it makes it look more like the "normal" backtrace.

Some fine details in the code are important for reducing
the amount of possible panic branches.
First, we reuse the `MAX_FRAMES` constant.

Co-authored-by: erikdesjardins <erikdesjardins@users.noreply.github.com>

Then we choose an arbitrary recursion depth for
the other case. In this case, I used 2d20. Honest.
…pected errors

then also use the new helper in a few other places
Previously, the test code would emit E0615, thus revealing the existence
of private methods that the programmer probably does not care about.
Now it ignores their existence instead, producing error E0609 (no field).

The motivating example is:

```rust
let x = std::rc::Rc::new(());
x.inner;
```

which would previously mention the private method `Rc::inner()`, even
though `Rc<T>` intentionally has no public methods so that it can be a
transparent smart pointer for any `T`.
…er-message, r=pnkfelix

Make SIGSEGV handler emit nicer backtraces

This annotates the code heavily with comments to explain what is going on, for the benefit of other compiler contributors. The backtrace also emits appropriate comments to clarify, to a programmer who may not know why a bunch of file paths and hexadecimal blather was just dumped into stderr, what is going on. Finally, it detects cycles and uses their regularity to avoid repeating a bunch of text. The previous backtraces we were emitting was extremely unfriendly, potentially confusing, and often alarming, and this makes things almost "nice".

We can't necessarily make them much nicer than this, because a signal handler must use "signal-safe" functions. This precludes conveniences like dynamic allocations. Fortunately, Rust's stdlib has allocation-free formatting, but it may hinder integrating this error with our localization middleware, as I wasn't able to clearly ascertain, at a glance, whether there was a zero-alloc path through it.

r? `@Nilstrieb`
parser: not insert dummy field in struct

Fixes rust-lang#114636

This PR eliminates the dummy field, initially introduced in rust-lang#113999, thereby enabling unrestricted use of `ident.unwrap()`. A side effect of this action is that we can only report the error of the first macro invocation field within the struct node.

An alternative solution might be giving a virtual name to the macro, but it appears more complex.(rust-lang#114636 (comment)). Furthermore, if you think rust-lang#114636 (comment) is a better solution, feel free to close this PR.
miri/diagnostics: don't forget to print_backtrace when ICEing on unexpected errors

This should fix the missing output encountered [here](rust-lang#115145 (comment)).

r? `@saethlin`
… r=compiler-errors

Make `get_return_block()` return `Some` only for HIR nodes in body

Fixes rust-lang#114918

The issue occurred while compiling the following input:

```rust
fn uwu() -> [(); { () }] {
    loop {}
}
```

It was caused by the code below trying to suggest a missing return type which resulted in a const eval cycle: https://github.com/rust-lang/rust/blob/1bd043098e05839afb557bd7a2858cb09a4054ca/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L68-L75

The root cause was `get_return_block()` returning an `Fn` node for a node in the return type (i.e. the second `()` in the return type `[(); { () }]` of the input) although it is supposed to do so only for nodes that lie in the body of the function and return `None` otherwise (at least as per my understanding).

The PR fixes the issue by fixing this behaviour of `get_return_block()`.
…=compiler-errors

suggest removing `impl` in generic trait bound position

rustc already does this recovery in type param position (`<T: impl Trait>` -> `<T: Trait>`).
This PR also adds that suggestion in trait bound position (e.g. `where T: impl Trait` or `trait Trait { type Assoc: impl Trait; }`)
new solver: handle edge case of a recursion limit of 0

Apparently a recursion limit of 0 is possible/valid/useful/used/cute, the more you know 🌟 .

(It's somewhat interesting to me that the old solver seemingly handles this, and that the new solver currently requires a recursion limit of 2 here)

r? `@compiler-errors.`

Fixes rust-lang#115351.
…-errors

Don't suggest adding parentheses to call an inaccessible method.

Previously, code of this form would emit E0615 (attempt to use a method as a field), thus emphasizing the existence of private methods that the programmer probably does not care about. Now it ignores their existence instead, producing error E0609 (no field). The motivating example is:

```rust
let x = std::rc::Rc::new(());
x.inner;
```

which would previously mention the private method `Rc::inner()`, even though `Rc<T>` intentionally has no public methods so that it can be a transparent smart pointer for any `T`.

```rust
error[E0615]: attempted to take value of method `inner` on type `Rc<()>`
 --> src/main.rs:3:3
  |
3 | x.inner;
  |   ^^^^^ method, not a field
  |
help: use parentheses to call the method
  |
3 | x.inner();
  |        ++
  ```

  With this change, it emits E0609 and no suggestion.
@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Aug 30, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Aug 30, 2023

📌 Commit ea23478 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 Aug 30, 2023
@bors
Copy link
Contributor

bors commented Aug 30, 2023

⌛ Testing commit ea23478 with merge 7659abc...

@bors
Copy link
Contributor

bors commented Aug 30, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 30, 2023
@bors bors merged commit 7659abc into rust-lang:master Aug 30, 2023
@rustbot rustbot added this to the 1.74.0 milestone Aug 30, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#113565 Make SIGSEGV handler emit nicer backtraces 0faa5a8bcd92d485a01ef969bcad9bf67c69c0b1 (link)
#114704 parser: not insert dummy field in struct 8a1ea6f871caf6aafd8965f8697bf6f72e2158d2 (link)
#115272 miri/diagnostics: don't forget to print_backtrace when ICEi… 37f6484355676be4604a56c47dad8afcbc5af387 (link)
#115313 Make get_return_block() return Some only for HIR nodes … 6dea558488a8e37a69664a41445bf732074107cd (link)
#115347 suggest removing impl in generic trait bound position 49650f8345d9fc15389ae0a6e44c2c6490927aef (link)
#115355 new solver: handle edge case of a recursion limit of 0 8cdb1c1f35732201f34f860e56346680423fe232 (link)
#115363 Don't suggest adding parentheses to call an inaccessible me… 42c8b378fe997a8d7793900311080b6acfdad10f (link)

previous master: 82c2eb48ee

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 (7659abc): 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

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

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: 630.718s -> 630.791s (0.01%)
Artifact size: 316.56 MiB -> 316.62 MiB (0.02%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 30, 2023
Fix bors missing a commit when merging rust-lang#115355

bors incorrectly merged an outdated version of PR rust-lang#115355 (via rollup rust-lang#115370):
- it [recorded r+](rust-lang#115355 (comment)) as approving commit rust-lang@325b585, and thus merged the original revision rust-lang@7762ac7
- but the branch at the time was at commit rust-lang@eefa07d, so bors missed the `compiler/rustc_trait_selection/src/solve/search_graph/mod.rs` cleanup in commit rust-lang@0e1e964 😓

Thankfully the change that bors missed was small, and this new PR corrects the situation (as I'd rather avoid having confusing multiple merge commits of PR rust-lang#115355 in the git history)

r? `@compiler-errors`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2023
Fix bors missing a commit when merging rust-lang#115355

bors incorrectly merged an outdated version of PR rust-lang#115355 (via rollup rust-lang#115370):
- it [recorded r+](rust-lang#115355 (comment)) as approving commit rust-lang@325b585, and thus merged the original revision rust-lang@7762ac7
- but the branch at the time was at commit rust-lang@eefa07d, so bors missed the `compiler/rustc_trait_selection/src/solve/search_graph/mod.rs` cleanup in commit rust-lang@0e1e964 😓

Thankfully the change that bors missed was small, and this new PR corrects the situation (as I'd rather avoid having confusing multiple merge commits of PR rust-lang#115355 in the git history)

r? ``@compiler-errors``
@matthiaskrgr matthiaskrgr deleted the rollup-l0e1zuj 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. 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.