-
-
Notifications
You must be signed in to change notification settings - Fork 134
Punct::new() docs promise a panic on invalid punctuation character, but no panic actually ever happens making it possible to create an invalid Punct #470
Description
See the docs for the Punct here. They are probably just copied from the original proc_macro::Punct type from the standard toolchain:
The
chargument must be a valid punctuation character permitted by the
language, otherwise the function will panic.
The problem here is that proc_macro2::Punct never actually panics on invalid input. It is defined literally as this:
Lines 828 to 841 in 9c1d3eb
| /// Creates a new `Punct` from the given character and spacing. | |
| /// | |
| /// The `ch` argument must be a valid punctuation character permitted by the | |
| /// language, otherwise the function will panic. | |
| /// | |
| /// The returned `Punct` will have the default span of `Span::call_site()` | |
| /// which can be further configured with the `set_span` method below. | |
| pub fn new(ch: char, spacing: Spacing) -> Self { | |
| Punct { | |
| ch, | |
| spacing, | |
| span: Span::call_site(), | |
| } | |
| } |
This allows for creating an invalid punctuation like this: Punct::new('{', Span::call_site()).
Why is this is problem?
It lead me to some unfortunate chain of debugging of the handling of incomplete syntax in Rust Analyzer. RA has yet another bug of its own (rust-lang/rust-analyzer#18244), where RA actually produces invalid proc_macro::Punct because it creates it using some unsafe ABI layout shenanigans. Then proc_macro2 happily converts such an invalid proc_macro::Punct into the proc_macro2::Punct equivalent, and that eventually leads to panic very late down the road (which is the actual pain) when one tries to run this code
Minimal reproduction of the late feedback about invalid punct. It leads to a panic in proc_macro2::TokenStream::extend():
// Code in proc-macro `my_lovely_macro/src/lib.rs` proc-macro crate
use proc_macro2::{Punct, Spacing, TokenStream, TokenTree};
#[proc_macro]
pub fn my_lovely_macro(_: proc_macro::TokenStream) -> proc_macro::TokenStream {
let punct = Punct::new('{', Spacing::Alone);
let token_stream2 = TokenStream::from_iter([TokenTree::Punct(punct)]);
// This innocent conversion panics!
proc_macro::TokenStream::from(token_stream2)
}// Code in runtime crate:
my_lovely_macro::my_lovely_macro! {}If you try to compile this code it panics with:
$ cargo build
Compiling my-lovely-macro v0.1.0 (/home/veetaha/sandbox/rust/crates/my-lovely-macro)
Compiling sandbox v0.1.0 (/home/veetaha/sandbox/rust/crates/sandbox)
error: proc macro panicked
--> crates/sandbox/src/main.rs:2:1
|
2 | my_lovely_macro::my_lovely_macro! {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: message: unsupported character `'{'`
This is because proc_macro::Punct::new() does actually have a panic on invalid characters (see code here).
Huge ❤️ for the proc_macro2/quote/syn ecosystem. It's a heroic design and implementation effort!