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 6 pull requests #134992

Merged
merged 17 commits into from
Jan 1, 2025
Merged

Rollup of 6 pull requests #134992

merged 17 commits into from
Jan 1, 2025

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Jan 1, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

mu001999 and others added 17 commits October 14, 2024 16:48
Ordinary code is much easier to work with than macro-generated code.
All of the tools that use this macro are currently in-tree, so support for
specifying a `SourceType` was not meaningfully used. It can potentially be
re-added in the future if needed.
This tricks rustfmt into formatting the macro arguments as expressions, instead
of giving up and ignoring them.
…elds have defaults

```
error: `Default` impl doesn't use the declared default field values
  --> $DIR/manual-default-impl-could-be-derived.rs:28:1
   |
LL | / impl Default for B {
LL | |     fn default() -> Self {
LL | |         B {
LL | |             x: s(),
   | |                --- this field has a default value
LL | |             y: 0,
   | |                - this field has a default value
...  |
LL | | }
   | |_^
   |
help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
   |
LL ~ #[derive(Default)] struct B {
   |
```

Note that above the structured suggestion also includes completely removing the manual `impl`, but the rendering doesn't.
…t, r=estebank

Remove allowing static_mut_refs lint
…bank

E0277: suggest dereferencing function arguments in more cases

This unifies and generalizes some of the logic in `TypeErrCtxt::suggest_dereferences` so that it will suggest dereferencing arguments to function/method calls in order to satisfy trait bounds in more cases.

Previously it would only fire on reference types, and it had two separate cases (one specifically to get through custom `Deref` impls when passing by-reference, and one specifically to catch rust-lang#87437). I've based the new checks loosely on what's done for `E0308` in `FnCtxt::suggest_deref_or_ref`: it will suggest dereferences to satisfy trait bounds whenever the referent is `Copy`, is boxed (& so can be moved out of the boxes), or is being passed by reference.

This doesn't make the suggestion fire in contexts other than function arguments or binary operators (which are in a separate case that this doesn't touch), and doesn't make it suggest a combination of `&`-removal and dereferences. Those would require a bit more restructuring, so I figured just doing this would be a decent first step.

Closes rust-lang#90997
…p-message, r=estebank

add suggestion for wrongly ordered format parameters

Add suggestion for wrongly ordered format parameters like `?#`.

Supersedes rust-lang#131004
Fix rust-lang#129966
…=estebank

Some small nits to the borrowck suggestions for mutating a map through index

1. Suggesting users to either use `.insert` or `.get_mut` (which do totally different things) can be a bit of a footgun, so let's make that a bit more nuanced.
2. I find the suggestion of `.get_mut(|val| { *val = whatever; })` to be a bit awkward. I changed this to be an if-let instead.
3. Fix a bug which was suppressing the structured suggestion for some mutations via the index operator on `HashMap`/`BTreeMap`.

r? estebank or reassign
bootstrap: Overhaul and simplify the `tool_check_step!` macro

Main changes:

- Pull most of `run` out of the macro and into a regular helper function
- Reduce the number of redundant/unnecessary macro arguments
- Switch to struct-like syntax so that optional arguments are clearer, and so that rustfmt is happy

~~The one “functional” change is that the `-check.stamp` files now get their name from the final path segment, instead of the struct name; in practice this means that they now contain more hyphens in some cases. As far as I'm aware, the exact filename doesn't matter so this should be fine.~~ (that change has been removed from this PR)
…iler-errors

Provide structured suggestion for `impl Default` of type where all fields have defaults

```
error: `Default` impl doesn't use the declared default field values
  --> $DIR/manual-default-impl-could-be-derived.rs:28:1
   |
LL | / impl Default for B {
LL | |     fn default() -> Self {
LL | |         B {
LL | |             x: s(),
   | |                --- this field has a default value
LL | |             y: 0,
   | |                - this field has a default value
...  |
LL | | }
   | |_^
   |
help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
   |
LL ~ #[derive(Default)] struct B {
   |
```

Note that above the structured suggestion also includes completely removing the manual `impl`, but the rendering doesn't.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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 Jan 1, 2025
@Zalathar
Copy link
Contributor Author

Zalathar commented Jan 1, 2025

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Jan 1, 2025

📌 Commit 65cb7c6 has been approved by Zalathar

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 1, 2025
@bors
Copy link
Contributor

bors commented Jan 1, 2025

⌛ Testing commit 65cb7c6 with merge bc3e301...

@bors
Copy link
Contributor

bors commented Jan 1, 2025

☀️ Test successful - checks-actions
Approved by: Zalathar
Pushing bc3e301 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 1, 2025
@bors bors merged commit bc3e301 into rust-lang:master Jan 1, 2025
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Jan 1, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#131439 Remove allowing static_mut_refs lint 0ca129abe6405fba48c82b93beef11aaaf1b3a27 (link)
#133292 E0277: suggest dereferencing function arguments in more cas… 826e2f2bcf807c2d593b0bd05d2dc1f2e041ef5e (link)
#134877 add suggestion for wrongly ordered format parameters 109df22886ddc40580f3824dc3105f3e9f3cb6e1 (link)
#134945 Some small nits to the borrowck suggestions for mutating a … e173dd4859e2dcdf4d5fd8943b9cddedbadb2dbe (link)
#134950 bootstrap: Overhaul and simplify the tool_check_step! mac… 21768616d09c092bc2b0cb2b0a830d94c91879bb (link)
#134979 Provide structured suggestion for impl Default of type wh… 3b106312323253b351b17458d898abfbfb1b2f8d (link)

previous master: a8953d83cf

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@Zalathar Zalathar deleted the rollup-pldy5w6 branch January 1, 2025 11:24
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bc3e301): 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)

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

Cycles

Results (secondary 1.9%)

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)
1.9% [1.9%, 1.9%] 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: 761.493s -> 762.453s (0.13%)
Artifact size: 325.53 MiB -> 325.56 MiB (0.01%)

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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

9 participants