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

Rustc pretty printer generates syntactically invalid output for some binary operators #98790

Closed
dtolnay opened this issue Jul 2, 2022 · 1 comment · Fixed by #119105
Closed
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jul 2, 2022

// repro.rs

macro_rules! repro {
    () => {
        match () {
            () => true,
        }
    };
}

pub fn repro() -> bool {
    repro!() | true
}
$ rustc --edition=2021 -Zunpretty=expanded lib.rs 
#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
macro_rules! repro { () => { match() { () => true, } } ; }

pub fn repro() -> bool { match () { () => true, } | true }

Another manifestation of the exact same pretty printer bug (in case this one is easier to incorporate into the test suite):

macro_rules! stringify_item {
    ($item:item) => {
        stringify!($item)
    };
}

macro_rules! repro {
    ($expr:expr) => {
        stringify_item! {
            pub fn repro() -> bool {
                $expr
            }
        }
    };
}

fn main() {
    println!("{}", repro!(match () { () => true } | true));
}
$ cargo run
pub fn repro() -> bool { match () { () => true, } | true }

The repro function in each of these outputs is syntactically invalid code, which is not what -Zunpretty=expanded or stringify! should be creating.

error: expected one of `...`, `..=`, `..`, `:`, or `|`, found `}`
 --> src/lib.rs:1:58
  |
1 | pub fn repro() -> bool { match () { () => true, } | true }
  |                                                          ^ expected one of `...`, `..=`, `..`, `:`, or `|`
  |
help: parentheses are required to parse this as an expression
  |
1 | pub fn repro() -> bool { (match () { () => true, }) | true }
  |                          +                        +

error[E0308]: mismatched types
 --> src/lib.rs:1:26
  |
1 | pub fn repro() -> bool { match () { () => true, } | true }
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^- help: consider using a semicolon here
  |                          |
  |                          expected `()`, found `bool`

The correct output in both cases would be either of the following syntactically valid outputs:

pub fn repro() -> bool { (match () { () => true, }) | true }
pub fn repro() -> bool { (match () { () => true, } | true) }

The rustc pretty printer already does parenthesis insertion in similar scenarios so it is a bug that it is not doing it here. For example:

// lib.rs

struct Struct {}

macro_rules! repro {
    () => {
        Struct {}
    };
}

pub fn repro() -> bool {
    match repro!() {
        _ => {}
    }
}
$ rustc --edition=2021 -Zunpretty=expanded src/lib.rs 
#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
struct Struct {}

macro_rules! repro { () => { Struct {} } ; }

pub fn repro() -> bool { match (Struct {}) { _ => {} } }

Notice that the pretty printer put parens around Struct {} because match Struct {} { _ => {} } would not have been valid syntax.

@dtolnay dtolnay added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 8, 2023
Force parentheses around `match` expression in binary expression

This attempts to solve rust-lang#98790.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 9, 2023
Force parentheses around `match` expression in binary expression

This attempts to solve rust-lang#98790.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 10, 2023
Force parentheses around `match` expression in binary expression

This attempts to solve rust-lang#98790.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 11, 2023
Force parentheses around `match` expression in binary expression

This attempts to solve rust-lang#98790.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 11, 2023
Force parentheses around `match` expression in binary expression

This attempts to solve rust-lang#98790.
@dtolnay
Copy link
Member Author

dtolnay commented Dec 19, 2023

The same bug with binary operators occurs not just in statement position, but also the RHS of match-arms.

macro_rules! stringify_expr {
    ($e:expr) => {
        stringify!($e)
    };
}

macro_rules! m {
    ($e:expr) => {
        stringify_expr!(match () { _ => $e })
    };
}

fn main() {
    println!("{}", m!({ 1 } - 1));
}
match () { _ => { 1 } - 1, }

The generated code is not valid Rust syntax.

error: unexpected `,` in pattern
 --> src/main.rs:2:30
  |
2 |     match () { _ => { 1 } - 1, }
  |                              ^
  |
help: try adding parentheses to match on a tuple...
  |
2 |     match () { _ => { 1 } (- 1,) }
  |                           +    +
help: ...or a vertical bar to match on multiple alternatives
  |
2 |     match () { _ => { 1 } - 1 | }
  |                           ~~~~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant