let_chains: Fix bugs in pretty printing.

Author: Centril

src/libsyntax/parse/parser.rs

@@ -41,7 +41,7 @@ use crate::parse::lexer::UnmatchedBrace;
 use crate::parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
 use crate::parse::token::{Token, TokenKind, DelimToken};
 use crate::parse::{new_sub_parser_from_file, ParseSess, Directory, DirectoryOwnership};
-use crate::util::parser::{AssocOp, Fixity};
+use crate::util::parser::{AssocOp, Fixity, prec_let_scrutinee_needs_par};
 use crate::print::pprust;
 use crate::ptr::P;
 use crate::parse::PResult;
@@ -3208,7 +3208,7 @@ impl<'a> Parser<'a> {
         self.expect(&token::Eq)?;
         let expr = self.with_res(
             Restrictions::NO_STRUCT_LITERAL,
-            |this| this.parse_assoc_expr_with(1 + AssocOp::LAnd.precedence(), None.into())
+            |this| this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into())
         )?;
         let span = lo.to(expr.span);
         self.sess.let_chains_spans.borrow_mut().push(span);

src/libsyntax/print/pprust.rs

@@ -1715,14 +1715,19 @@ impl<'a> State<'a> {
         self.ann.post(self, AnnNode::Block(blk))
     }
 
+    /// Print a `let pats = scrutinee` expression.
     pub fn print_let(&mut self, pats: &[P<ast::Pat>], scrutinee: &ast::Expr) -> io::Result<()> {
         self.s.word("let ")?;
 
         self.print_pats(pats)?;
         self.s.space()?;
 
         self.word_space("=")?;
-        self.print_expr_as_cond(scrutinee)
+        self.print_expr_cond_paren(
+            scrutinee,
+            Self::cond_needs_par(scrutinee)
+            || parser::needs_par_as_let_scrutinee(scrutinee.precedence().order())
+        )
     }
 
     fn print_else(&mut self, els: Option<&ast::Expr>) -> io::Result<()> {
@@ -1794,30 +1799,30 @@ impl<'a> State<'a> {
     }
 
     pub fn print_expr_maybe_paren(&mut self, expr: &ast::Expr, prec: i8) -> io::Result<()> {
-        let needs_par = expr.precedence().order() < prec;
-        if needs_par {
-            self.popen()?;
-        }
-        self.print_expr(expr)?;
-        if needs_par {
-            self.pclose()?;
-        }
-        Ok(())
+        self.print_expr_cond_paren(expr, expr.precedence().order() < prec)
     }
 
     /// Print an expr using syntax that's acceptable in a condition position, such as the `cond` in
     /// `if cond { ... }`.
     pub fn print_expr_as_cond(&mut self, expr: &ast::Expr) -> io::Result<()> {
-        let needs_par = match expr.node {
+        self.print_expr_cond_paren(expr, Self::cond_needs_par(expr))
+    }
+
+    /// Does `expr` need parenthesis when printed in a condition position?
+    fn cond_needs_par(expr: &ast::Expr) -> bool {
+        match expr.node {
             // These cases need parens due to the parse error observed in #26461: `if return {}`
             // parses as the erroneous construct `if (return {})`, not `if (return) {}`.
             ast::ExprKind::Closure(..) |
             ast::ExprKind::Ret(..) |
             ast::ExprKind::Break(..) => true,
 
             _ => parser::contains_exterior_struct_lit(expr),
-        };
+        }
+    }
 
+    /// Print `expr` or `(expr)` when `needs_par` holds.
+    fn print_expr_cond_paren(&mut self, expr: &ast::Expr, needs_par: bool) -> io::Result<()> {
         if needs_par {
             self.popen()?;
         }
@@ -1949,6 +1954,17 @@ impl<'a> State<'a> {
             // of `(x as i32) < ...`. We need to convince it _not_ to do that.
             (&ast::ExprKind::Cast { .. }, ast::BinOpKind::Lt) |
             (&ast::ExprKind::Cast { .. }, ast::BinOpKind::Shl) => parser::PREC_FORCE_PAREN,
+            // We are given `(let _ = a) OP b`.
+            //
+            // - When `OP <= LAnd` we should print `let _ = a OP b` to avoid redundant parens
+            //   as the parser will interpret this as `(let _ = a) OP b`.
+            //
+            // - Otherwise, e.g. when we have `(let a = b) < c` in AST,
+            //   parens are required since the parser would interpret `let a = b < c` as
+            //   `let a = (b < c)`. To achieve this, we force parens.
+            (&ast::ExprKind::Let { .. }, _) if !parser::needs_par_as_let_scrutinee(prec) => {
+                parser::PREC_FORCE_PAREN
+            }
             _ => left_prec,
         };
 

src/libsyntax/util/parser.rs

@@ -318,6 +318,9 @@ impl ExprPrecedence {
             ExprPrecedence::Box |
             ExprPrecedence::AddrOf |
             // Here `let pats = expr` has `let pats =` as a "unary" prefix of `expr`.
+            // However, this is not exactly right. When `let _ = a` is the LHS of a binop we
+            // need parens sometimes. E.g. we can print `(let _ = a) && b` as `let _ = a && b`
+            // but we need to print `(let _ = a) < b` as-is with parens.
             ExprPrecedence::Let |
             ExprPrecedence::Unary => PREC_PREFIX,
 
@@ -352,6 +355,19 @@ impl ExprPrecedence {
     }
 }
 
+/// In `let p = e`, operators with precedence `<=` this one requires parenthesis in `e`.
+crate fn prec_let_scrutinee_needs_par() -> usize {
+    AssocOp::LAnd.precedence()
+}
+
+/// Suppose we have `let _ = e` and the `order` of `e`.
+/// Is the `order` such that `e` in `let _ = e` needs parenthesis when it is on the RHS?
+///
+/// Conversely, suppose that we have `(let _ = a) OP b` and `order` is that of `OP`.
+/// Can we print this as `let _ = a OP b`?
+crate fn needs_par_as_let_scrutinee(order: i8) -> bool {
+    order <= prec_let_scrutinee_needs_par() as i8
+}
 
 /// Expressions that syntactically contain an "exterior" struct literal i.e., not surrounded by any
 /// parens or other delimiters, e.g., `X { y: 1 }`, `X { y: 1 }.method()`, `foo == X { y: 1 }` and