about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-12-21 01:30:15 +0100
committerGitHub <noreply@github.com>2024-12-21 01:30:15 +0100
commitf3b19f54fa9ab80e309001412fc8bcaa4fd767ee (patch)
treed1961d6d9a9023da5f1fa0cd27d80e7b374fbe2b
parent472bbb9f0caeaf9222b54baf3782df0e2c433c1c (diff)
parentfe06c5dce1db564d42678ea8e4f4a8ae451fe4a3 (diff)
downloadrust-f3b19f54fa9ab80e309001412fc8bcaa4fd767ee.tar.gz
rust-f3b19f54fa9ab80e309001412fc8bcaa4fd767ee.zip
Rollup merge of #133782 - dtolnay:closuresjumps, r=spastorino,traviscross
Precedence improvements: closures and jumps

This PR fixes some cases where rustc's pretty printers would redundantly parenthesize expressions that didn't need it.

<table>
<tr><th>Before</th><th>After</th></tr>
<tr><td><code>return (|x: i32| x)</code></td><td><code>return |x: i32| x</code></td></tr>
<tr><td><code>(|| -> &mut () { std::process::abort() }).clone()</code></td><td><code>|| -> &mut () { std::process::abort() }.clone()</code></td></tr>
<tr><td><code>(continue) + 1</code></td><td><code>continue + 1</code></td></tr>
</table>

Tested by `echo "fn main() { let _ = $AFTER; }" | rustc -Zunpretty=expanded /dev/stdin`.

The pretty-printer aims to render the syntax tree as it actually exists in rustc, as faithfully as possible, in Rust syntax. It can insert parentheses where forced by Rust's grammar in order to preserve the meaning of a macro-generated syntax tree, for example in the case of `a * $rhs` where $rhs is `b + c`. But for any expression parsed from source code, without a macro involved, there should never be a reason for inserting additional parentheses not present in the original.

For closures and jumps (return, break, continue, yield, do yeet, become) the unneeded parentheses came from the precedence of some of these expressions being misidentified. In the same order as the table above:

- Jumps and closures are supposed to have equal precedence. The [Rust Reference](https://doc.rust-lang.org/1.83.0/reference/expressions.html#expression-precedence) says so, and in Syn they do. There is no Rust syntax that would require making a precedence distinction between jumps and closures. But in rustc these were previously 2 distinct levels with the closure being lower, hence the parentheses around a closure inside a jump (but not a jump inside a closure).

- When a closure is written with an explicit return type, the grammar [requires](https://doc.rust-lang.org/1.83.0/reference/expressions/closure-expr.html) that the closure body consists of exactly one block expression, not any other arbitrary expression as usual for closures. Parsing of the closure body does not continue after the block expression. So in `|| { 0 }.clone()` the clone is inside the closure body and applies to `{ 0 }`, whereas in `|| -> _ { 0 }.clone()` the clone is outside and applies to the closure as a whole.

- Continue never needs parentheses. It was previously marked as having the lowest possible precedence but it should have been the highest, next to paths and loops and function calls, not next to jumps.
-rw-r--r--compiler/rustc_ast/src/ast.rs11
-rw-r--r--compiler/rustc_ast/src/util/parser.rs3
-rw-r--r--compiler/rustc_hir/src/hir.rs13
-rw-r--r--tests/ui-fulldeps/pprust-parenthesis-insertion.rs6
4 files changed, 24 insertions, 9 deletions
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
index 69c3e0553d4..cec868e5c8e 100644
--- a/compiler/rustc_ast/src/ast.rs
+++ b/compiler/rustc_ast/src/ast.rs
@@ -1322,11 +1322,15 @@ impl Expr {
     }
 
     pub fn precedence(&self) -> ExprPrecedence {
-        match self.kind {
-            ExprKind::Closure(..) => ExprPrecedence::Closure,
+        match &self.kind {
+            ExprKind::Closure(closure) => {
+                match closure.fn_decl.output {
+                    FnRetTy::Default(_) => ExprPrecedence::Jump,
+                    FnRetTy::Ty(_) => ExprPrecedence::Unambiguous,
+                }
+            }
 
             ExprKind::Break(..)
-            | ExprKind::Continue(..)
             | ExprKind::Ret(..)
             | ExprKind::Yield(..)
             | ExprKind::Yeet(..)
@@ -1360,6 +1364,7 @@ impl Expr {
             | ExprKind::Block(..)
             | ExprKind::Call(..)
             | ExprKind::ConstBlock(_)
+            | ExprKind::Continue(..)
             | ExprKind::Field(..)
             | ExprKind::ForLoop { .. }
             | ExprKind::FormatArgs(..)
diff --git a/compiler/rustc_ast/src/util/parser.rs b/compiler/rustc_ast/src/util/parser.rs
index 0d8042005a8..1d4b01aa94c 100644
--- a/compiler/rustc_ast/src/util/parser.rs
+++ b/compiler/rustc_ast/src/util/parser.rs
@@ -231,8 +231,7 @@ impl AssocOp {
 
 #[derive(Clone, Copy, PartialEq, PartialOrd)]
 pub enum ExprPrecedence {
-    Closure,
-    // return, break, yield
+    // return, break, yield, closures
     Jump,
     // = += -= *= /= %= &= |= ^= <<= >>=
     Assign,
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index 398b694ae6b..8cea269f298 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -1943,11 +1943,15 @@ pub struct Expr<'hir> {
 
 impl Expr<'_> {
     pub fn precedence(&self) -> ExprPrecedence {
-        match self.kind {
-            ExprKind::Closure { .. } => ExprPrecedence::Closure,
+        match &self.kind {
+            ExprKind::Closure(closure) => {
+                match closure.fn_decl.output {
+                    FnRetTy::DefaultReturn(_) => ExprPrecedence::Jump,
+                    FnRetTy::Return(_) => ExprPrecedence::Unambiguous,
+                }
+            }
 
             ExprKind::Break(..)
-            | ExprKind::Continue(..)
             | ExprKind::Ret(..)
             | ExprKind::Yield(..)
             | ExprKind::Become(..) => ExprPrecedence::Jump,
@@ -1973,6 +1977,7 @@ impl Expr<'_> {
             | ExprKind::Block(..)
             | ExprKind::Call(..)
             | ExprKind::ConstBlock(_)
+            | ExprKind::Continue(..)
             | ExprKind::Field(..)
             | ExprKind::If(..)
             | ExprKind::Index(..)
@@ -1990,7 +1995,7 @@ impl Expr<'_> {
             | ExprKind::UnsafeBinderCast(..)
             | ExprKind::Err(_) => ExprPrecedence::Unambiguous,
 
-            ExprKind::DropTemps(ref expr, ..) => expr.precedence(),
+            ExprKind::DropTemps(expr, ..) => expr.precedence(),
         }
     }
 
diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs
index b83e576076d..184458bad55 100644
--- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs
+++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs
@@ -68,6 +68,12 @@ static EXPRS: &[&str] = &[
     // These mean different things.
     "return - 2",
     "(return) - 2",
+    // Closures and jumps have equal precedence.
+    "|| return break 2",
+    "return break || 2",
+    // Closures with a return type have especially high precedence.
+    "|| -> T { x } + 1",
+    "(|| { x }) + 1",
     // These mean different things.
     "if let _ = true && false {}",
     "if let _ = (true && false) {}",