From e4d124daf616e0e7ba3a8981061c1bea88c09c46 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 21 Sep 2026 16:16:52 +0100 Subject: [PATCH 1/3] internal: handle item and expr diagnostics differently Currently `DiagCtxt` is used to allow error recovery in the pin-init macros, by generating both a `compile_error!()` macro item and continue expansion, and then merge token streams together. This works very well for items, however for expressions, this will produce invalid expression and result in a confusing error message. error: macro expansion ignores `::` and any tokens following --> tests/ui/compile-fail/zeroable/invalid_spread.rs:15:9 | 13 | let _ = init!(Foo { | _____________- 14 | | a: 0, 15 | | ..MyZeroable::init_zeroed() | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 16 | | }); | |______- caused by the macro expansion here | = note: the usage of `init!` is likely invalid in expression context Fix this by wrap the concatenated diagnostics in a block, with the generated expression in its tail position. This results in the desired error message. error: expected nothing or `..Zeroable::init_zeroed()`. --> tests/ui/compile-fail/zeroable/invalid_spread.rs:15:9 | 15 | ..MyZeroable::init_zeroed() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Signed-off-by: Gary Guo --- internal/src/diagnostics.rs | 55 ++++++++++++++++--- internal/src/lib.rs | 12 ++-- .../compile-fail/zeroable/invalid_spread.rs | 17 ++++++ .../zeroable/invalid_spread.stderr | 19 +++++++ 4 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 tests/ui/compile-fail/zeroable/invalid_spread.rs create mode 100644 tests/ui/compile-fail/zeroable/invalid_spread.stderr diff --git a/internal/src/diagnostics.rs b/internal/src/diagnostics.rs index c7d9b3e6..c42f1095 100644 --- a/internal/src/diagnostics.rs +++ b/internal/src/diagnostics.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use proc_macro2::TokenStream; -use quote::quote_spanned; +use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, Error}; pub(crate) struct DiagCtxt(TokenStream); @@ -29,16 +29,55 @@ impl DiagCtxt { )); } - pub(crate) fn with( - fun: impl FnOnce(&mut DiagCtxt) -> Result, + fn with( + f: impl FnOnce(&mut DiagCtxt) -> Result, + merge_diag: impl FnOnce(TokenStream, TokenStream) -> TokenStream, + convert_diag: impl FnOnce(TokenStream) -> TokenStream, ) -> TokenStream { let mut dcx = Self(TokenStream::new()); - match fun(&mut dcx) { - Ok(mut stream) => { - stream.extend(dcx.0); - stream + match f(&mut dcx) { + Ok(stream) => { + if dcx.0.is_empty() { + stream + } else { + merge_diag(stream, dcx.0) + } } - Err(ErrorGuaranteed(())) => dcx.0, + Err(ErrorGuaranteed(())) => convert_diag(dcx.0), } } + + pub(crate) fn for_item( + f: impl FnOnce(&mut DiagCtxt) -> Result, + ) -> TokenStream { + Self::with( + f, + |mut out, diag| { + out.extend(diag); + out + }, + std::convert::identity, + ) + } + + pub(crate) fn for_expr( + f: impl FnOnce(&mut DiagCtxt) -> Result, + ) -> TokenStream { + Self::with( + f, + |out, diag| { + // Diagnostics that we generate are always items. + // So for expressions create a block to place diagnostics in item position. + quote!({ + #diag + #out + }) + }, + |diag| { + quote!({ + #diag + }) + }, + ) + } } diff --git a/internal/src/lib.rs b/internal/src/lib.rs index c488019d..0410024b 100644 --- a/internal/src/lib.rs +++ b/internal/src/lib.rs @@ -25,31 +25,31 @@ mod zeroable; pub fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream { let args = parse_macro_input!(args); let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| pin_data::pin_data(args, input, dcx)).into() + DiagCtxt::for_item(|dcx| pin_data::pin_data(args, input, dcx)).into() } #[proc_macro_attribute] pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream { let args = parse_macro_input!(args); let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| pinned_drop::pinned_drop(args, input, dcx)).into() + DiagCtxt::for_item(|dcx| pinned_drop::pinned_drop(args, input, dcx)).into() } #[proc_macro_derive(Zeroable)] pub fn derive_zeroable(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| zeroable::derive(input, dcx)).into() + DiagCtxt::for_item(|dcx| zeroable::derive(input, dcx)).into() } #[proc_macro_derive(MaybeZeroable)] pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| zeroable::maybe_derive(input, dcx)).into() + DiagCtxt::for_item(|dcx| zeroable::maybe_derive(input, dcx)).into() } #[proc_macro] pub fn init(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| { + DiagCtxt::for_expr(|dcx| { init::expand_with_cfg(input, Some("::core::convert::Infallible"), false, dcx) }) .into() @@ -58,7 +58,7 @@ pub fn init(input: TokenStream) -> TokenStream { #[proc_macro] pub fn pin_init(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input); - DiagCtxt::with(|dcx| { + DiagCtxt::for_expr(|dcx| { init::expand_with_cfg(input, Some("::core::convert::Infallible"), true, dcx) }) .into() diff --git a/tests/ui/compile-fail/zeroable/invalid_spread.rs b/tests/ui/compile-fail/zeroable/invalid_spread.rs new file mode 100644 index 00000000..515ad7ed --- /dev/null +++ b/tests/ui/compile-fail/zeroable/invalid_spread.rs @@ -0,0 +1,17 @@ +extern crate pin_init; +use pin_init::*; + +use Zeroable as MyZeroable; + +#[derive(Zeroable)] +struct Foo { + a: usize, + b: usize, +} + +fn main() { + let _ = init!(Foo { + a: 0, + ..MyZeroable::init_zeroed() + }); +} diff --git a/tests/ui/compile-fail/zeroable/invalid_spread.stderr b/tests/ui/compile-fail/zeroable/invalid_spread.stderr new file mode 100644 index 00000000..f07ce3a7 --- /dev/null +++ b/tests/ui/compile-fail/zeroable/invalid_spread.stderr @@ -0,0 +1,19 @@ +error: expected nothing or `..Zeroable::init_zeroed()`. + --> tests/ui/compile-fail/zeroable/invalid_spread.rs:15:9 + | +15 | ..MyZeroable::init_zeroed() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: unused import: `Zeroable as MyZeroable` + --> tests/ui/compile-fail/zeroable/invalid_spread.rs:4:5 + | +4 | use Zeroable as MyZeroable; + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + +error[E0063]: missing field `b` in initializer of `Foo` + --> tests/ui/compile-fail/zeroable/invalid_spread.rs:13:19 + | +13 | let _ = init!(Foo { + | ^^^ missing `b` From cfb0806f94d426b31ec9d5908f006955e3e15b90 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 21 Sep 2026 15:31:16 +0100 Subject: [PATCH 2/3] internal: make `DiagCtxt` available inside parser Currently, there is a split between `syn::Error` and pin-init's custom `DiagCtxt`. Parsing has no access to `DiagCtxt` and therefore cannot report diagnostics without failing the parse. Bridge the gap by making the `DiagCtxt` available inside parsers using `thread_local!`. `parse_macro_input!` returns error as token stream directly, so it is no longer suitable when the parsing is moved inside `DiagCtxt` closure. Add a `From for ErrorGuaranteed` implementation so `?` can be used to add a `syn::Error` into the current diagnostic context and obtain a `ErrorGuaranteed`. Clean up the handler of using `<-` inside tuple expression as a demonstration of the usefulness of having `DiagCtxt` access inside `Parse`. Signed-off-by: Gary Guo --- internal/src/diagnostics.rs | 84 ++++++++++++++++++++++++++++--------- internal/src/init.rs | 41 +++++++----------- internal/src/lib.rs | 32 +++++++------- 3 files changed, 96 insertions(+), 61 deletions(-) diff --git a/internal/src/diagnostics.rs b/internal/src/diagnostics.rs index c42f1095..efdcf45f 100644 --- a/internal/src/diagnostics.rs +++ b/internal/src/diagnostics.rs @@ -1,32 +1,68 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT +use std::cell::RefCell; use std::fmt::Display; +use std::marker::PhantomData; use proc_macro2::TokenStream; use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, Error}; -pub(crate) struct DiagCtxt(TokenStream); +pub(crate) struct DiagCtxt(PhantomData<*mut ()>); pub(crate) struct ErrorGuaranteed(()); +struct DiagCtxtData { + diag: TokenStream, +} + +thread_local! { + static DIAGNOSTICS: RefCell> = const { RefCell::new(None) }; +} + +// Allows `syn::Error` to be emitted into the current diagnostic context with just `?`. +impl From for ErrorGuaranteed { + fn from(error: syn::Error) -> Self { + DIAGNOSTICS.with_borrow_mut(|data| { + data.as_mut() + .unwrap() + .diag + .extend(error.into_compile_error()); + }); + Self(()) + } +} + impl DiagCtxt { - pub(crate) fn error(&mut self, span: impl Spanned, msg: impl Display) -> ErrorGuaranteed { - let error = Error::new(span.span(), msg); - self.0.extend(error.into_compile_error()); - ErrorGuaranteed(()) + pub(crate) fn error(&self, span: impl Spanned, msg: impl Display) -> ErrorGuaranteed { + Error::new(span.span(), msg).into() } - pub(crate) fn warn(&mut self, span: impl Spanned, msg: impl Display) { + pub(crate) fn warn(&self, span: impl Spanned, msg: impl Display) { // Have the message start on a new line for visual clarity. let msg = format!("\n{}", msg); - self.0.extend(quote_spanned!(span.span() => - // Approximate using deprecated warning while `proc_macro_diagnostic` is unstable. - const _: () = { - #[deprecated = #msg] - const fn warn() {} - warn(); - }; - )); + DIAGNOSTICS.with_borrow_mut(|data| { + data.as_mut() + .unwrap() + .diag + .extend(quote_spanned!(span.span() => + // Approximate using deprecated warning while `proc_macro_diagnostic` is + // unstable. + const _: () = { + #[deprecated = #msg] + const fn warn() {} + warn(); + }; + )) + }); + } + + /// Execute the provided function with the current diagnostic context. + pub(crate) fn current(f: impl FnOnce(&DiagCtxt) -> R) -> R { + DIAGNOSTICS.with_borrow(|data| { + assert!(data.is_some(), "No active `DiagCtxt`"); + }); + + f(&DiagCtxt(PhantomData)) } fn with( @@ -34,16 +70,26 @@ impl DiagCtxt { merge_diag: impl FnOnce(TokenStream, TokenStream) -> TokenStream, convert_diag: impl FnOnce(TokenStream) -> TokenStream, ) -> TokenStream { - let mut dcx = Self(TokenStream::new()); - match f(&mut dcx) { + DIAGNOSTICS.with_borrow_mut(|data| { + assert!(data.is_none(), "`DiagCtxt` cannot be nested"); + *data = Some(DiagCtxtData { + diag: TokenStream::new(), + }); + }); + + let result = f(&mut DiagCtxt(PhantomData)); + + let data = DIAGNOSTICS.with_borrow_mut(|data| data.take().unwrap()); + + match result { Ok(stream) => { - if dcx.0.is_empty() { + if data.diag.is_empty() { stream } else { - merge_diag(stream, dcx.0) + merge_diag(stream, data.diag) } } - Err(ErrorGuaranteed(())) => convert_diag(dcx.0), + Err(ErrorGuaranteed(())) => convert_diag(data.diag), } } diff --git a/internal/src/init.rs b/internal/src/init.rs index e98182c6..c5b5a0ce 100644 --- a/internal/src/init.rs +++ b/internal/src/init.rs @@ -44,9 +44,6 @@ pub(crate) enum InitExprKind { struct InitTupleField { attrs: Vec, - /// `<-` is not valid in constructor syntax; it is parsed anyway so that it can be rejected - /// with a proper diagnostic instead of a parse error. - left_arrow_token: Option, value: Expr, } @@ -84,20 +81,6 @@ impl InitExprTuple { rest: None, } } - - fn validate(&self, dcx: &mut DiagCtxt) -> Result<(), ErrorGuaranteed> { - let mut result = Ok(()); - for field in &self.fields { - if let Some(left_arrow_token) = &field.left_arrow_token { - result = Err(dcx.error( - left_arrow_token, - "`<-` is not supported in tuple constructor syntax; name the fields by index \ - instead, e.g. `Type { 0 <- initializer, 1: value }`", - )); - } - } - result - } } struct This { @@ -153,8 +136,6 @@ pub(crate) fn expand_with_cfg( ) -> Result { let initializer = match initializer.kind { InitExprKind::Tuple(expr) => { - expr.validate(dcx)?; - let mut initializer = Initializer { attrs: initializer.attrs, this: initializer.this, @@ -579,9 +560,20 @@ impl InitExprTuple { let paren_token = parenthesized!(content in input); let mut fields = Punctuated::new(); while !content.is_empty() { + let attrs = content.call(Attribute::parse_outer)?; + + if let Some(left_arrow_token) = content.parse::>()? { + DiagCtxt::current(|dcx| { + dcx.error( + left_arrow_token, + "`<-` is not supported in tuple constructor syntax; name the fields by \ + index instead, e.g. `Type { 0 <- initializer, 1: value }`", + ) + }); + } + fields.push_value(InitTupleField { - attrs: content.call(Attribute::parse_outer)?, - left_arrow_token: content.parse()?, + attrs, value: content.parse()?, }); if content.is_empty() { @@ -758,13 +750,8 @@ impl ToTokens for InitExprTuple { impl ToTokens for InitTupleField { fn to_tokens(&self, tokens: &mut TokenStream) { - let Self { - attrs, - left_arrow_token, - value, - } = self; + let Self { attrs, value } = self; tokens.append_all(attrs); - left_arrow_token.to_tokens(tokens); value.to_tokens(tokens); } } diff --git a/internal/src/lib.rs b/internal/src/lib.rs index 0410024b..07b3d33d 100644 --- a/internal/src/lib.rs +++ b/internal/src/lib.rs @@ -10,7 +10,6 @@ #![allow(missing_docs)] use proc_macro::TokenStream; -use syn::parse_macro_input; use crate::diagnostics::DiagCtxt; @@ -23,43 +22,46 @@ mod zeroable; #[proc_macro_attribute] pub fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream { - let args = parse_macro_input!(args); - let input = parse_macro_input!(input); - DiagCtxt::for_item(|dcx| pin_data::pin_data(args, input, dcx)).into() + DiagCtxt::for_item(|dcx| pin_data::pin_data(syn::parse(args)?, syn::parse(input)?, dcx)).into() } #[proc_macro_attribute] pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream { - let args = parse_macro_input!(args); - let input = parse_macro_input!(input); - DiagCtxt::for_item(|dcx| pinned_drop::pinned_drop(args, input, dcx)).into() + DiagCtxt::for_item(|dcx| pinned_drop::pinned_drop(syn::parse(args)?, syn::parse(input)?, dcx)) + .into() } #[proc_macro_derive(Zeroable)] pub fn derive_zeroable(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input); - DiagCtxt::for_item(|dcx| zeroable::derive(input, dcx)).into() + DiagCtxt::for_item(|dcx| zeroable::derive(syn::parse(input)?, dcx)).into() } #[proc_macro_derive(MaybeZeroable)] pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input); - DiagCtxt::for_item(|dcx| zeroable::maybe_derive(input, dcx)).into() + DiagCtxt::for_item(|dcx| zeroable::maybe_derive(syn::parse(input)?, dcx)).into() } #[proc_macro] pub fn init(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input); DiagCtxt::for_expr(|dcx| { - init::expand_with_cfg(input, Some("::core::convert::Infallible"), false, dcx) + init::expand_with_cfg( + syn::parse(input)?, + Some("::core::convert::Infallible"), + false, + dcx, + ) }) .into() } #[proc_macro] pub fn pin_init(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input); DiagCtxt::for_expr(|dcx| { - init::expand_with_cfg(input, Some("::core::convert::Infallible"), true, dcx) + init::expand_with_cfg( + syn::parse(input)?, + Some("::core::convert::Infallible"), + true, + dcx, + ) }) .into() } From 66741d1c1e132d27431ad8fa940eeeb012b4d742 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 21 Sep 2026 19:33:51 +0100 Subject: [PATCH 3/3] internal: improve diagnostics robustness against panicking Currently, if proc macro panicked, the diagnostics clean up is not executed, and further invocation will cause the "DiagCtxt cannot be nested" error. While we should aim to have no panics inside proc macros, producing a sensible diagnostics message even when macro panicked is very useful for developing. Thus, catch proc macro panics and convert them to errors, and emit them together with all diagnostics accumulated so far. Ideally we would like panic location w/ line numbers being available as well; however this is not currently implementable without overriding the global panic hook. Signed-off-by: Gary Guo --- internal/src/diagnostics.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/internal/src/diagnostics.rs b/internal/src/diagnostics.rs index efdcf45f..e88e520e 100644 --- a/internal/src/diagnostics.rs +++ b/internal/src/diagnostics.rs @@ -4,7 +4,7 @@ use std::cell::RefCell; use std::fmt::Display; use std::marker::PhantomData; -use proc_macro2::TokenStream; +use proc_macro2::{Span, TokenStream}; use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, Error}; @@ -77,7 +77,27 @@ impl DiagCtxt { }); }); - let result = f(&mut DiagCtxt(PhantomData)); + let mut dcx = DiagCtxt(PhantomData); + let result = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(&mut dcx))) { + Ok(result) => result, + Err(payload) => { + // Robustness against panicking in macros. + // + // Ensure that any error messages are still emitted when this happens. + let message = if let Some(&s) = payload.downcast_ref::<&'static str>() { + s + } else if let Some(s) = payload.downcast_ref::() { + s.as_str() + } else { + "Box" + }; + + Err(dcx.error( + Span::mixed_site(), + format!("proc macro panicked: {message}"), + )) + } + }; let data = DIAGNOSTICS.with_borrow_mut(|data| data.take().unwrap());