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

Keep formatting around literals with weird types #4901

Closed
programmerjake opened this issue Jul 15, 2021 · 8 comments
Closed

Keep formatting around literals with weird types #4901

programmerjake opened this issue Jul 15, 2021 · 8 comments

Comments

@programmerjake
Copy link
Member

I'm writing a Hardware Description Language (HDL) embedded in Rust and I'm using a proc-macro val! to convert Rust expressions (syn::Expr) to the code needed for generating my compiler IR. rustfmt guesses that the macro arguments are expressions and nicely formats them:

counter.assign_data_in(val!(if overflowed { 0 } else { counter_output + 1 }));

However, I support integer types with less usual bit counts, such as u20. rustfmt refuses to format as soon as it sees an odd integer literal, so I have to manually format that section of my code:
https://salsa.debian.org/Kazan-team/rust-hdl/-/blob/f232063e797ddb097d88df4e6580c7ee861efdef/examples/blinky.rs#L17

let overflowed = val!(counter_output >= 1_000_000_u20);

however syn has no problems whatsoever parsing that expression.

Please adjust rustfmt to still format expressions even if it can't parse the type of a literal.

Short demo (builds fine):
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=3da9013aa7b1b43706ad584e652897c5

macro_rules! val {
    ($($tokens:tt)*) => {
        0 // good enough for demo
    };
}

fn f() -> i32 {
    // not formatted by rustfmt
    val!(a >
    123u20)
}

fn g() -> i32 {
    // formatted by rustfmt
    val!(a >
    123u32)
}

formats to:

macro_rules! val {
    ($($tokens:tt)*) => {
        0 // good enough for demo
    };
}

fn f() -> i32 {
    // not formatted by rustfmt
    val!(a >
    123u20)
}

fn g() -> i32 {
    // formatted by rustfmt
    val!(a > 123u32)
}
@calebcartwright
Copy link
Member

Closing as a functional duplicate of #8

rustfmt's support for macros, and macro calls in particular, is still rather weak. As you've noted, in instances of calls with paren and bracket delims rustfmt can format args that are normal rust code. This happens because rustfmt will see if the rustc parser can handle the tokens and parse out an expr, type, etc. which are things rustfmt knows how to format; otherwise rustfmt simply has to use the original snippet from the associated span.

@programmerjake
Copy link
Member Author

What I'm asking for is for the expression/literal parser/formatter to keep working around unexpected literals, this shouldn't require any modifications to the existing macro handling code. Therefore, I don't think it's functionally a duplicate of #8. This would also work where there's no macros in sight:

fn f() -> u20 {
    1 + 3u20
}

@calebcartwright
Copy link
Member

What I'm asking for is for the expression/literal parser/formatter to keep working around unexpected literals

rustfmt uses rustc's parser. If rustc can't parse it, then there's nothing we can do with it.

Therefore, I don't think it's functionally a duplicate of #8

You're welcome to disagree of course, but I can assure you that it is. rustfmt cannot do any more with non-parseable macro args until we have a holistic solution for doing so, and we're tracking that under #8

This would also work where there's no macros in sight:

Perhaps I'm misunderstanding you, but there's no way we could format this. In order to do so we'd need a practical rewrite of rustfmt that would either consume a different parser/ast that could handle that snippet, build our own parser/ast, or switching to an entirely token-based formatter (which would be a complete rewrite).

@programmerjake
Copy link
Member Author

I'm saying modify rustc's parser so it still produces an expression ast (like syn can) that can be formatted

@programmerjake
Copy link
Member Author

that should be relatively easy, just change the Lit AST node to also have a BadLit variant, or something like that.

@programmerjake
Copy link
Member Author

Supporting unrecognized literals also makes rustfmt more future-proof, it allows using an old version of rustfmt to format code for a newer version of Rust that added a new kind of literal.

@calebcartwright
Copy link
Member

I'm saying modify rustc's parser so it still produces an expression ast (like syn can) that can be formatted
that should be relatively easy, just change the Lit AST node to also have a BadLit variant, or something like that.

That's something you'd have to take up with the compiler team to gauge their willingness. We don't have any control over what rust's parser will accept.

@programmerjake
Copy link
Member Author

Opened rust-lang/rust#87243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants