-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Error spans into imported macros render poorly #20923
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
Comments
cc @kmcallister |
I don't think copy-pasting from the source file is an acceptable solution for a production compiler. We need to strip comments at the very least. I do want to solve this problem in some way, maybe just a better pretty-printer for TTs. |
What exactly is the downside of copy-pasting from a source file? Why would we need to strip comments? |
I am becoming increasingly concerned about the current approach. A correctness issue have been discovered with the current approach: #20701. It's been papered over by adding a heuristic to the token tree pretty printer to tell it to make its output of arbitrary token tree sequences look more like Rust source. I am not at all convinced that this is the only correctness issue present. How confident are we that making the token tree pretty printer output Rust won't break token trees that aren't supposed to represent Rust source code? Is the error output that the compiler currently displays for these kinds of errors in macro expansion acceptable in a production compiler? Are we planning on rewriting and fixing the token tree pretty printer in the next month and a half? As I see it, there is a large set of Bad Things that happen due to the current serialization approach, and the plan for correcting those Bad Things is fuzzy at best. Could you please identify a concrete Bad Thing that happens with the old approach? As far as I can tell, all we're doing right now is applying a lossy anti-compression algorithm to macro sources. |
I think it's very surprising if comments in your source code can make it into the compiled binary. Someone will leak something sensitive or embarrassing and it'll be real bad for them and for us. Also I think it's weird to include source code solely for the purpose of better error reporting. But — we do lots of weird things; that's not a very strong argument against. I'd believe that this is the only workable solution with macros. I don't know offhand what other compilers for languages with macros do. The fact that we need character-accurate source to reconstruct a token tree seems like a massive deficiency in the Rust lexer/parser, and maybe the grammar itself? But I agree there's little chance of that being fixed for 1.0. Really I wanted to use a binary serialization of token trees, much as we do for generics & cross-crate inlining, but in light of the error reporting issue I don't think that's workable. Anyway I didn't expect the pretty-printing approach to have such severe downsides. Perhaps the right approach is to strip comments directly from the character stream, and maybe normalize whitespace if it produces better errors. I'm confident I can get that done in time for the beta release. This ticket is high on my list for macro reform v2. |
The old approach may have licensing issues as well. Is a compiled |
I'm not going to have time to work on this for 1.0-final, sorry. If you want to switch back to including source, that's fine by me as long as it's clearly documented. |
This seems much nicer today: error: no method named `write_fmt` found for type `&mut {integer}` in the current scope
--> foo.rs:2:5
|
2 | write!(&mut 2, "");
| ^^^^^^^^^^^^^^^^^^^
|
= note: this error originates in a macro outside of the current crate
error: aborting due to previous error @sfackler what do you think? |
👍 |
Exported macros are currently serialized as token trees and pretty printed for display, and the pretty printer is not very good. Macros were previously serialized as a string of their source definition for this exact reason.
Reexported macro definitions in rustdoc have the same problem: http://doc.rust-lang.org/std/macro.write!.html
The text was updated successfully, but these errors were encountered: