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

proc_macro types Display impls don’t respect the input layout #56474

Open
hadronized opened this issue Dec 3, 2018 · 6 comments
Open

proc_macro types Display impls don’t respect the input layout #56474

hadronized opened this issue Dec 3, 2018 · 6 comments
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)

Comments

@hadronized
Copy link

Hey there,

I’m not sure whether it’s the expected behavior of the Display implementations for token types in proc_macro, but the Display implementations lose positional information. I made a blog post about it and release a crate to fix that problem, proc-macro-faithful-display.

I posted that on Discord and @eddyb advised me to open an issue here. So here it is. Not sure whether:

  • There’s an actual problem, because perhaps the Display implementors have never meant to provide positional correctness.
  • If there’s a problem:
    • Should we assume the current behavior of Display correct and instead provide another type to perform the faithful display (like the .display() function for &Path).
    • Or instead just fix Display.

\o/

@eddyb
Copy link
Member

eddyb commented Dec 3, 2018

IMO we should, at least, provide some functionality, on Span or elsewhere, to get at that information that is being lost by looking at only the tokens.

cc @alexcrichton @dtolnay

@hadronized
Copy link
Author

If you’re interested, here’s my implementation for reference.

@alexcrichton
Copy link
Member

This seems fine to me to add an an unstable basis to the proc_macro crate, and it also seems fine by me to improve the default output of Display wherever possible! We just need to be sure to not regress anything as usual!

@eddyb
Copy link
Member

eddyb commented Dec 4, 2018

I'm a bit worried changing fmt::Display might break various tools that don't expect arbitrary whitespace and/or comments between tokens.

@alexcrichton
Copy link
Member

In discussions in the past @dtolnay and I have felt that any changes to Display which produce still-valid-Rust-code is fine to make. Just like Debug in the standard library we don't guarantee an exact output, only that it conforms to a set of guidelines, and for TokenStream it's that it always produces valid Rust code.

In that sense if we update displayed output and parsers break we'd consider it bugs in the parsers rather than the compiler, although we would of course work with the crates if there's enough fallout.

@eddyb
Copy link
Member

eddyb commented Dec 6, 2018

This is an alternative, I suppose: #55780.

@jonas-schievink jonas-schievink added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Jan 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..)
Projects
None yet
Development

No branches or pull requests

4 participants