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

Merged
merged 17 commits into from
Dec 21, 2024
Merged

Rollup of 7 pull requests #134605

merged 17 commits into from
Dec 21, 2024

Conversation

jhpratt
Copy link
Member

@jhpratt jhpratt commented Dec 21, 2024

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

hkBst and others added 17 commits December 20, 2024 18:55
On parse errors where an ident is found where one wasn't expected, see if the next elements might have been meant as method call or field access.

```
error: expected one of `.`, `;`, `?`, `else`, or an operator, found `map`
  --> $DIR/missing-dot-on-statement-expression.rs:7:29
   |
LL |     let _ = [1, 2, 3].iter()map(|x| x);
   |                             ^^^ expected one of `.`, `;`, `?`, `else`, or an operator
   |
help: you might have meant to write a method call
   |
LL |     let _ = [1, 2, 3].iter().map(|x| x);
   |                             +
```
Detect missing `.` in method chain in `let` bindings and statements

On parse errors where an ident is found where one wasn't expected, see if the next elements might have been meant as method call or field access.

```
error: expected one of `.`, `;`, `?`, `else`, or an operator, found `map`
  --> $DIR/missing-dot-on-statement-expression.rs:7:29
   |
LL |     let _ = [1, 2, 3].iter()map(|x| x);
   |                             ^^^ expected one of `.`, `;`, `?`, `else`, or an operator
   |
help: you might have meant to write a method call
   |
LL |     let _ = [1, 2, 3].iter().map(|x| x);
   |                             +
```
…nikomatsakis

Handle `DropKind::ForLint` in coroutines correctly

Fixes rust-lang#134566
Fixes rust-lang#134541
Improve prose around basic examples of Iter and IterMut
Improve prose around `as_slice` example of Iter
Improve prose around into_slice example of IterMut

Having a part without modification and one with seems redundant, since `into_slice` is only called for the part without. I have brought the modification into the remaining part, although it perhaps does not add much (or only distracts?).
Less unwrap() in documentation

I think the common use of `.unwrap()` in examples makes it overrepresented, looking like a more typical way of error handling than it really is in real programs.

Therefore, this PR changes a bunch of examples to use different error handling methods, primarily the `?` operator. Additionally, `unwrap()` docs warn that it might abort the program.
Fix parenthesization of chained comparisons by pretty-printer

Example:

```rust
macro_rules! repro {
    () => {
        1 < 2
    };
}

fn main() {
    let _ = repro!() == false;
}
```

Previously `-Zunpretty=expanded` would pretty-print this syntactically invalid output: `fn main() { let _ = 1 < 2 == false; }`

```console
error: comparison operators cannot be chained
 --> <anon>:8:23
  |
8 | fn main() { let _ = 1 < 2 == false; }
  |                       ^   ^^
  |
help: parenthesize the comparison
  |
8 | fn main() { let _ = (1 < 2) == false; }
  |                     +     +
```

With the fix, it will print `fn main() { let _ = (1 < 2) == false; }`.

Making `-Zunpretty=expanded` consistently produce syntactically valid Rust output is important because that is what makes it possible for `cargo expand` to format and perform filtering on the expanded code.

## Review notes

According to `rg '\.fixity\(\)' compiler/` the `fixity` function is called only 3 places:

- /~https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L283-L287

- /~https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_hir_pretty/src/lib.rs#L1295-L1299

- /~https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_parse/src/parser/expr.rs#L282-L289

The 2 pretty printers definitely want to treat comparisons using `Fixity::None`. That's the whole bug being fixed. Meanwhile, the parser's `Fixity::None` codepath is previously unreachable as indicated by the comment, so as long as `Fixity::None` here behaves exactly the way that `Fixity::Left` used to behave, you can tell that this PR definitely does not constitute any behavior change for the parser.

My guess for why comparison operators were set to `Fixity::Left` instead of `Fixity::None` is that it's a very old workaround for giving a good chained comparisons diagnostic (like what I pasted above). Nowadays that is handled by a different dedicated codepath.
@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 Dec 21, 2024
@jhpratt
Copy link
Member Author

jhpratt commented Dec 21, 2024

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Dec 21, 2024

📌 Commit ea8bc3b has been approved by jhpratt

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 Dec 21, 2024
@bors
Copy link
Contributor

bors commented Dec 21, 2024

⌛ Testing commit ea8bc3b with merge 6076bee...

@bors
Copy link
Contributor

bors commented Dec 21, 2024

☀️ Test successful - checks-actions
Approved by: jhpratt
Pushing 6076bee to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 21, 2024
@bors bors merged commit 6076bee into rust-lang:master Dec 21, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 21, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#133087 Detect missing . in method chain in let bindings and st… c9e17e93463ce0e1166d826b8823e71b1a46071d (link)
#134575 Handle DropKind::ForLint in coroutines correctly faafbacf4a0577226db2af755b11995d5f5c12f2 (link)
#134576 Improve prose around basic examples of Iter and IterMut fc167e0361fa264500ad0f02365a84d3d3e31156 (link)
#134577 Improve prose around as_slice example of Iter 78c9a96ba5c8cff9af1ffa151a01bf83f9fa1f7c (link)
#134579 Improve prose around into_slice example of IterMut d0cbba38e5da2923e509034c1d551de1d008d5b5 (link)
#134593 Less unwrap() in documentation 44b9a9450efd687d830e68fbd3722046a16afcf8 (link)
#134600 Fix parenthesization of chained comparisons by pretty-print… 199d17a018164d8746e6fae8424579a83fb8bbfd (link)

previous master: 73c278fd93

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 (6076bee): 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 (primary -1.7%, secondary 4.5%)

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)
4.5% [4.5%, 4.5%] 1
Improvements ✅
(primary)
-1.7% [-1.7%, -1.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.7% [-1.7%, -1.7%] 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: 766.302s -> 767s (0.09%)
Artifact size: 330.23 MiB -> 330.21 MiB (-0.00%)

@jhpratt jhpratt deleted the rollup-quiss71 branch December 27, 2024 20:29
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.

9 participants