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 4 pull requests #130498

Merged
merged 8 commits into from
Sep 18, 2024
Merged

Rollup of 4 pull requests #130498

merged 8 commits into from
Sep 18, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

veera-sivarajan and others added 8 commits September 12, 2024 21:27
…, r=chenyukang

Implement a Method to Seal `DiagInner`'s Suggestions

This PR adds a method on `DiagInner` called `.seal_suggestions()` to prevent new suggestions from being added while preserving existing suggestions.

This is useful because currently there is no way to prevent new suggestions from being added to a diagnostic. `.disable_suggestions()` is the closest but it gets rid of all suggestions before and after the call.

Therefore, `.seal_suggestions()` can be used when, for example, misspelled keyword is detected and reported. In such cases, we may want to prevent other suggestions from being added to the diagnostic, as they would likely be meaningless once the misspelled keyword is identified. For context: rust-lang#129899 (comment)

To store an additional state, the type of the `suggestions` field in `DiagInner` was changed into a three variant enum. While this change affects files across different crates, care was taken to preserve the existing code's semantics. This is validated by the fact that all UI tests pass without any modifications.

r? chenyukang
…youxu

Ensure that `keyword_ident` lint doesn't trigger on `'r#kw` lifetime

Fixes rust-lang#130486
…rrors

more crash tests

r? `@compiler-errors`
Fix circular fn_sig queries to correct number of args for methods

Fixes rust-lang#130400. This was a [debug assert](https://github.com/rust-lang/rust/blob/28e8f01c2a2f33fb4214925a704e3223b372cad5/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L2557) added to some argument error reporting code in rust-lang#129320 which verified that the number of params (from the HIR) matched the `matched_inputs` which ultimately come from ty::FnSig. In the reduced test case:

```
fn foo(&mut self) -> _ {
    foo()
}
```

There is a circular dependency computing the ty::FnSig -- when trying to compute it, we try to figure out the return value, which again depends on this ty::FnSig. In rust-lang#105162, this was supported by short-circuiting the cycle by synthesizing a FnSig with error types for parameters. The [code in question](https://github.com/rust-lang/rust/pull/105162/files#diff-a65feec6bfffb19fbdc60a80becd1030c82a56c16b177182cd277478fdb04592R44) computes the number of parameters by taking the number of parameters from the hir::FnDecl and adding 1 if there is an implicit self parameter.

I might be missing a subtlety here, but AFAICT the adjustment for implicit self args is unnecessary and results in one too many args. For example, for this non-errorful code:

```
trait Foo {
    fn bar(&self) {}
}
```

The resulting hir::FnDecl and ty::FnSig both have the same number of inputs -- 1. So, this PR removes that adjustment and adds a test for the debug ICE.

r? `@compiler-errors`
@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. rollup A PR which is a rollup labels Sep 18, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Contributor

bors commented Sep 18, 2024

📌 Commit c52d58d 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 Sep 18, 2024
@bors
Copy link
Contributor

bors commented Sep 18, 2024

⌛ Testing commit c52d58d with merge f6bcd09...

@bors
Copy link
Contributor

bors commented Sep 18, 2024

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

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#130116 Implement a Method to Seal DiagInner's Suggestions a3e29a637f30015f0b29ae25522ac746873dc4d5 (link)
#130489 Ensure that keyword_ident lint doesn't trigger on 'r#kw 520e93e6123ad88641c712504e381714e22b7b35 (link)
#130491 more crash tests da0daa6394843f75bda70800800f863a45a8ebdb (link)
#130496 Fix circular fn_sig queries to correct number of args for m… d4c80016106d061480b968c5411f57b03923dd7d (link)

previous master: 60c3673456

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 (f6bcd09): 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 (secondary 2.7%)

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

Cycles

Results (secondary 2.4%)

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

Binary size

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

Bootstrap: 768.777s -> 767.306s (-0.19%)
Artifact size: 341.28 MiB -> 341.25 MiB (-0.01%)

@matthiaskrgr matthiaskrgr deleted the rollup-tg4d0zi 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
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants