pprust: fix parenthesization of exprs

[spernsteiner]

The pretty printer in syntax::print::pprust currently relies on the presence of ExprKind::Paren hints in order to correctly parenthesize expressions in its output. If Paren nodes are missing, it sometimes produces wrong output, such as printing 1 - (2 - 3) as 1 - 2 - 3. This PR fixes pprust to correctly print expressions regardless of the presence or absence of Paren nodes. This should make pprust easier to use with programmatically constructed ASTs.

A few notes:

• I added a function for assigning precedence values to exprs in syntax::util::parser, since there is already code there for assigning precedence values to binops. Let me know if I should move this somewhere more pprust-specific.

• I also moved the contains_exterior_struct_lit function from rustc_lint::unused::UnusedParens into syntax::util::parser, since it's needed for determining the correct parenthesization of if/while conditional expressions.

• I couldn't find a good way to compare two exprs for equivalence while ignoring semantically-irrelevant details like spans. So the test for the new behavior relies on a slight hack: it adds Paren nodes everywhere, so that the pretty-printed version exactly reflects the structure of the AST, and then compares the printed strings. This works, but let me know if there's a better way.

[rust-highfive]

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

[aidanhs]

Thanks for the PR @epdtry! We'll check in now and again to make sure @pnkfelix or another reviewer gets to this soon.

[petrochenkov]

Looks good to me.
I didn't figure out how exactly the test works though.
I assume this was tested on some real autogenerated code as well? (There are some quite non-obvious cases that are treated correctly.)

There's also HIR pretty-printing in librustc/hir/print.rs which is an almost exact copy of AST pretty-printing in libsyntax/print/pprust.rs.
Could you reapply the changes to that file as well?
HIR doesn't have any information about parens, so HIR pretty-printing would benefit from this patch even more.

[arielb1]

Hi @epdtry! I think @petrochenkov has some concerns about your PR. Could you try to address them? Just a friendly ping to make sure this isn't getting lost

[spernsteiner]

Hi, sorry about the delay. I've written some new commits to address @petrochenkov's comments, but I did the work for this PR "on the clock", so there's a review process the changes need to go through before I can publish them anywhere. This will hopefully be done soon, but it could be up to another week or two. I'll update this PR with the new commits as soon as I get the approval.

[spernsteiner]

@petrochenkov I've added similar parenthesization fixes to the HIR printer. This part doesn't have a test case, because I'm not sure how to write one. (The tricks I used for testing the AST printer don't work easily because there's no HIR parser and no HIR Paren nodes.) I also added a comment to the pprust test case, to better explain how the test is meant to work.

[spernsteiner]

Rebased (adding support for ExprYield) to fix the previous travis build error

[spernsteiner]

Github says there was an error, but I don't see any errors on the travis build page, only some xcode builds that were cancelled midway through cloning the repo. Can I ignore this, or is something actually broken?

[petrochenkov]

The Travis error looks spurious.

@bors r+
Thanks! This is a great patch.

[bors]

📌 Commit 347db06 has been approved by petrochenkov

[bors]

⌛ Testing commit 347db06 with merge 6de8f2b23b0f876de69fbc48dac8ff776ab5e4fe...

[bors]

💔 Test failed - status-appveyor

[kennytm]

check-aux x86_64-pc-windows-msvc failed, pretty\issue-4264.rs formatted differently. Legit.

Error message

failures:
---- [pretty] pretty\issue-4264.rs stdout ----
	
error: pretty-printed source does not match expected source
expected:
------------------------------------------
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std as std;
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// pretty-compare-only
// pretty-mode:hir,typed
// pp-exact:issue-4264.pp
// #4264 fixed-length vector types
pub fn foo(_: [i32; (3 as usize)]) ({ } as ())
pub fn bar() ({
                  const FOO: usize = ((5 as usize) - (4 as usize) as usize);
                  let _: [(); (FOO as usize)] = ([(() as ())] as [(); 1]);
                  let _: [(); (1 as usize)] = ([(() as ())] as [(); 1]);
                  let _ =
                      (((&([(1 as i32), (2 as i32), (3 as i32)] as [i32; 3])
                            as &[i32; 3]) as *const _ as *const [i32; 3]) as
                          *const [i32; (3 as usize)] as *const [i32; 3]);
                  ((::fmt::format as
                       fn(std::fmt::Arguments<'_>) -> std::string::String {std::fmt::format})(((<::std::fmt::Arguments>::new_v1
                                                                                                   as
                                                                                                   fn(&[&str], &[std::fmt::ArgumentV1<'_>]) -> std::fmt::Arguments<'_> {std::fmt::Arguments<'_>::new_v1})(({
                                                                                                                                                                                                               static __STATIC_FMTSTR:
                                                                                                                                                                                                                      &'static [&'static str]
                                                                                                                                                                                                                      =
                                                                                                                                                                                                                   (&([("test"
                                                                                                                                                                                                                           as
                                                                                                                                                                                                                           &'static str)]
                                                                                                                                                                                                                         as
                                                                                                                                                                                                                         [&'static str; 1])
                                                                                                                                                                                                                       as
                                                                                                                                                                                                                       &'static [&'static str; 1]);
                                                                                                                                                                                                               (__STATIC_FMTSTR
                                                                                                                                                                                                                   as
                                                                                                                                                                                                                   &'static [&'static str])
                                                                                                                                                                                                           }
                                                                                                                                                                                                              as
                                                                                                                                                                                                              &[&str]),
                                                                                                                                                                                                          (&(match (()
                                                                                                                                                                                                                       as
                                                                                                                                                                                                                       ())
                                                                                                                                                                                                                 {
                                                                                                                                                                                                                 ()
                                                                                                                                                                                                                 =>
                                                                                                                                                                                                                 ([]
                                                                                                                                                                                                                     as
                                                                                                                                                                                                                     [std::fmt::ArgumentV1<'_>; 0]),
                                                                                                                                                                                                             }
                                                                                                                                                                                                                as
                                                                                                                                                                                                                [std::fmt::ArgumentV1<'_>; 0])
                                                                                                                                                                                                              as
                                                                                                                                                                                                              &[std::fmt::ArgumentV1<'_>; 0]))
                                                                                                  as
                                                                                                  std::fmt::Arguments<'_>))
                      as std::string::String);
              } as ())
pub type Foo = [i32; (3 as usize)];
pub struct Bar {
    pub x: [i32; (3 as usize)],
}
pub struct TupleBar([i32; (4 as usize)]);
pub enum Baz { BazVariant([i32; (5 as usize)]), }
pub fn id<T>(x: T) -> T ({ (x as T) } as T)
pub fn use_id() ({
                     let _ =
                         ((id::<[i32; (3 as usize)]> as
                              fn([i32; 3]) -> [i32; 3] {id::<[i32; 3]>})(([(1
                                                                               as
                                                                               i32),
                                                                           (2
                                                                               as
                                                                               i32),
                                                                           (3
                                                                               as
                                                                               i32)]
                                                                             as
                                                                             [i32; 3]))
                             as [i32; 3]);
                 } as ())
fn main() ({ } as ())
------------------------------------------
actual:
------------------------------------------
#[prelude_import]
use std::prelude::v1::*;
#[macro_use]
extern crate std as std;
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// pretty-compare-only
// pretty-mode:hir,typed
// pp-exact:issue-4264.pp
// #4264 fixed-length vector types
pub fn foo(_: [i32; (3 as usize)]) ({ } as ())
pub fn bar() ({
                  const FOO: usize = ((5 as usize) - (4 as usize) as usize);
                  let _: [(); (FOO as usize)] = ([(() as ())] as [(); 1]);
                  let _: [(); (1 as usize)] = ([(() as ())] as [(); 1]);
                  let _ =
                      (((&([(1 as i32), (2 as i32), (3 as i32)] as [i32; 3])
                            as &[i32; 3]) as *const _ as *const [i32; 3]) as
                          *const [i32; (3 as usize)] as *const [i32; 3]);
                  ((::fmt::format as
                       fn(std::fmt::Arguments<'_>) -> std::string::String {std::fmt::format})(((<::std::fmt::Arguments>::new_v1
                                                                                                   as
                                                                                                   fn(&[&str], &[std::fmt::ArgumentV1<'_>]) -> std::fmt::Arguments<'_> {std::fmt::Arguments<'_>::new_v1})(({
                                                                                                                                                                                                               static __STATIC_FMTSTR:
                                                                                                                                                                                                                      &'static [&'static str]
                                                                                                                                                                                                                      =
                                                                                                                                                                                                                   (&([("test"
                                                                                                                                                                                                                           as
                                                                                                                                                                                                                           &'static str)]
                                                                                                                                                                                                                         as
                                                                                                                                                                                                                         [&'static str; 1])
                                                                                                                                                                                                                       as
                                                                                                                                                                                                                       &'static [&'static str; 1]);
                                                                                                                                                                                                               (__STATIC_FMTSTR
                                                                                                                                                                                                                   as
                                                                                                                                                                                                                   &'static [&'static str])
                                                                                                                                                                                                           }
                                                                                                                                                                                                              as
                                                                                                                                                                                                              &[&str]),
                                                                                                                                                                                                          (&((match (()
                                                                                                                                                                                                                        as
                                                                                                                                                                                                                        ())
                                                                                                                                                                                                                  {
                                                                                                                                                                                                                  ()
                                                                                                                                                                                                                  =>
                                                                                                                                                                                                                  ([]
                                                                                                                                                                                                                      as
                                                                                                                                                                                                                      [std::fmt::ArgumentV1<'_>; 0]),
                                                                                                                                                                                                              }
                                                                                                                                                                                                                 as
                                                                                                                                                                                                                 [std::fmt::ArgumentV1<'_>; 0]))
                                                                                                                                                                                                              as
                                                                                                                                                                                                              &[std::fmt::ArgumentV1<'_>; 0]))
                                                                                                  as
                                                                                                  std::fmt::Arguments<'_>))
                      as std::string::String);
              } as ())
pub type Foo = [i32; (3 as usize)];
pub struct Bar {
    pub x: [i32; (3 as usize)],
}
pub struct TupleBar([i32; (4 as usize)]);
pub enum Baz { BazVariant([i32; (5 as usize)]), }
pub fn id<T>(x: T) -> T ({ (x as T) } as T)
pub fn use_id() ({
                     let _ =
                         ((id::<[i32; (3 as usize)]> as
                              fn([i32; 3]) -> [i32; 3] {id::<[i32; 3]>})(([(1
                                                                               as
                                                                               i32),
                                                                           (2
                                                                               as
                                                                               i32),
                                                                           (3
                                                                               as
                                                                               i32)]
                                                                             as
                                                                             [i32; 3]))
                             as [i32; 3]);
                 } as ())
fn main() ({ } as ())
------------------------------------------
thread '[pretty] pretty\issue-4264.rs' panicked at 'explicit panic', src\tools\compiletest\src\runtest.rs:376:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
    [pretty] pretty\issue-4264.rs

Diff (ignoring whitespace)

--- 1.rs	2017-09-07 15:14:53.000000000 +0800
+++ 2.rs	2017-09-07 15:15:06.000000000 +0800
@@ -44,7 +44,7 @@
                                                                                                                                                                                                            }
                                                                                                                                                                                                               as
                                                                                                                                                                                                               &[&str]),
-                                                                                                                                                                                                          (&(match (()
+                                                                                                                                                                                                          (&((match (()
                                                                                                                                                                                                                        as
                                                                                                                                                                                                                        ())
                                                                                                                                                                                                                  {
@@ -55,7 +55,7 @@
                                                                                                                                                                                                                      [std::fmt::ArgumentV1<'_>; 0]),
                                                                                                                                                                                                              }
                                                                                                                                                                                                                 as
-                                                                                                                                                                                                                [std::fmt::ArgumentV1<'_>; 0])
+                                                                                                                                                                                                                 [std::fmt::ArgumentV1<'_>; 0]))
                                                                                                                                                                                                               as
                                                                                                                                                                                                               &[std::fmt::ArgumentV1<'_>; 0]))
                                                                                                   as

[spernsteiner]

Should be fixed in the latest commit. I turned up the precedence of the block-like exprs ({}, if, while, etc) so that &match ... no longer gets needlessly parenthesized as &(match ...).

[petrochenkov]

@bors r+

[bors]

📌 Commit 3454d99 has been approved by petrochenkov

[bors]

⌛ Testing commit 3454d99 with merge fd20f603855e10430b2cf9b06793f5c854c603c3...

[bors]

💔 Test failed - status-travis

[kennytm]

@bors retry

Travis's Macs are not feeling well this week. https://www.traviscistatus.com/incidents/4qylrqvy50gy

[bors]

⌛ Testing commit 3454d99 with merge 7eefbffdd79d1f1b18395bb578a607a2647c6b82...

[bors]

💔 Test failed - status-travis

[spernsteiner]

I see failures in run-make/sanitizer-address and run-make/sanitizer-dylib-link. I can't imagine how those tests could be affected by this PR, so is this another travis bug of some kind? If not, how should I start debugging? I've run those tests locally and they both pass.

[petrochenkov]

@bors retry

[bors]

⌛ Testing commit 3454d99 with merge cd398828b8d7e44c534eabcfd198dbe82007debc...

[petrochenkov]

Travis is probably going to fail again due to the issue addressed by #44398 and #44399

[alexcrichton]

@bors: retry

• prioritizing travis: Downgrade to previous images temporarily #44399

[bors]

⌛ Testing commit 3454d99 with merge 57fe99b7a42ae25b3ea22f1bf37786c531cbbb56...

[bors]

💔 Test failed - status-appveyor

[kennytm]

@bors retry check i686-pc-windows-msvc 3 hour timed out.

[bors]

⌛ Testing commit 3454d99 with merge d6ad402...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing d6ad402 to master...