Skip to content

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
Collaborator

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
Collaborator

bors commented Dec 21, 2024

⌛ Testing commit ea8bc3b with merge 6076bee...

@bors
Copy link
Collaborator

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