-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Reduce unnecessary escaping in proc_macro::Literal::character/string #95343
Conversation
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
cc #60495 |
don't know the areas actually impacted by this change, so |
I'd like to use this opportunity to address #60495 (maybe not to make all the relevant code changes, but at least to make a decision).
@dtolnay |
I don't have a preference between these. They both come out equally readable in typical usage, from what I've seen. The fact that proc macros and macro_rules do the opposite is a good catch. Here is what I tried: // main.rs
macro_rules! decl {
(#[doc = $tt:tt]) => {
println!("decl macro: #[doc = {}]", stringify!($tt));
};
}
fn main() {
decl! {
///"
}
proc! {
///"
}
} // lib.rs
#[proc_macro]
pub fn proc(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
if let proc_macro::TokenTree::Group(group) = input.into_iter().nth(1).unwrap() {
println!("proc macro: #[doc = {}]", group.stream().into_iter().nth(2).unwrap());
}
proc_macro::TokenStream::new()
} proc macro: #[doc = "\""]
decl macro: #[doc = r#"""#] I think it would be reasonable to make these two behave the same. It's not important to me which behavior we pick.
I think you got it right in your response to this above.
From what I can tell, this divergence was introduced by #83079. The current behavior of |
So one more alternative is to introduce a new At least it will be predictable in that case, unlike debug escaping. |
Ok, I think this is good to go. |
📌 Commit f383134 has been approved by |
Rollup of 7 pull requests Successful merges: - rust-lang#92942 (stabilize windows_process_extensions_raw_arg) - rust-lang#94817 (Release notes for 1.60.0) - rust-lang#95343 (Reduce unnecessary escaping in proc_macro::Literal::character/string) - rust-lang#95431 (Stabilize total_cmp) - rust-lang#95438 (Add SyncUnsafeCell.) - rust-lang#95467 (Windows: Synchronize asynchronous pipe reads and writes) - rust-lang#95609 (Suggest borrowing when trying to coerce unsized type into `dyn Trait`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I noticed that https://doc.rust-lang.org/proc_macro/struct.Literal.html#method.character is producing unreadable literals that make macro-expanded code unnecessarily hard to read. Since the proc macro server was using
escape_unicode()
, every char is escaped using\u{…}
regardless of whether there is any need to do so. For exampleLiteral::character('=')
would previously produce'\u{3d}'
which unnecessarily obscures the meaning when reading the macro-expanded code.I've changed Literal::string also in this PR because
str
'sDebug
impl is also smarter than just callingescape_debug
on every char. For exampleLiteral::string("ferris's")
would previously produce"ferris\'s"
but will now produce"ferris's"
.