From c27aff51a3438896dd2cf850cea35d869f02542f Mon Sep 17 00:00:00 2001 From: Jon C Date: Mon, 5 Aug 2024 20:50:15 +0200 Subject: [PATCH 1/4] program-error: Add option to specify solana_program crate Problem There are going to be a lot of issues with people seeing errors on spl_program_error derivation. When a build pulls in two version of solana_program, the macro uses `::solana_program`, which now becomes ambiguous. Summary of changes I'm not sure if this is a good idea, but this PR gives the ability to specify which version of `solana_program` to use when deriving the various traits on your error type. Borsh has this same functionality, and it's saved us when pulling in multiple versions of borsh in the SDK. Note: this PR defaults to `solana_program` instead of `::solana_program`, which might cause downstream issues. --- .../program-error/derive/src/macro_impl.rs | 39 +++++---- libraries/program-error/derive/src/parser.rs | 80 ++++++++++++------- libraries/program-error/tests/spl.rs | 17 ++++ libraries/type-length-value/src/error.rs | 5 +- 4 files changed, 97 insertions(+), 44 deletions(-) diff --git a/libraries/program-error/derive/src/macro_impl.rs b/libraries/program-error/derive/src/macro_impl.rs index 21e89ee3441..172fc6ecc18 100644 --- a/libraries/program-error/derive/src/macro_impl.rs +++ b/libraries/program-error/derive/src/macro_impl.rs @@ -7,7 +7,7 @@ use { sha2::{Digest, Sha256}, syn::{ punctuated::Punctuated, token::Comma, Expr, ExprLit, Ident, ItemEnum, Lit, LitInt, LitStr, - Token, Variant, + Path, Token, Variant, }, }; @@ -36,10 +36,16 @@ pub enum MacroType { impl MacroType { /// Generates the corresponding tokens based on variant selection pub fn generate_tokens(&mut self) -> proc_macro2::TokenStream { + let default_solana_program_crate_path = + Ident::new("solana_program", Span::call_site()).into(); match self { - Self::IntoProgramError { ident } => into_program_error(ident), - Self::DecodeError { ident } => decode_error(ident), - Self::PrintProgramError { ident, variants } => print_program_error(ident, variants), + Self::IntoProgramError { ident } => { + into_program_error(ident, &default_solana_program_crate_path) + } + Self::DecodeError { ident } => decode_error(ident, &default_solana_program_crate_path), + Self::PrintProgramError { ident, variants } => { + print_program_error(ident, variants, &default_solana_program_crate_path) + } Self::SplProgramError { args, item_enum } => spl_program_error(args, item_enum), } } @@ -48,20 +54,20 @@ impl MacroType { /// Builds the implementation of /// `Into` More specifically, /// implements `From for solana_program::program_error::ProgramError` -pub fn into_program_error(ident: &Ident) -> proc_macro2::TokenStream { +pub fn into_program_error(ident: &Ident, solana_program_crate: &Path) -> proc_macro2::TokenStream { quote! { - impl From<#ident> for ::solana_program::program_error::ProgramError { + impl From<#ident> for #solana_program_crate::program_error::ProgramError { fn from(e: #ident) -> Self { - ::solana_program::program_error::ProgramError::Custom(e as u32) + #solana_program_crate::program_error::ProgramError::Custom(e as u32) } } } } /// Builds the implementation of `solana_program::decode_error::DecodeError` -pub fn decode_error(ident: &Ident) -> proc_macro2::TokenStream { +pub fn decode_error(ident: &Ident, solana_program_crate: &Path) -> proc_macro2::TokenStream { quote! { - impl ::solana_program::decode_error::DecodeError for #ident { + impl #solana_program_crate::decode_error::DecodeError for #ident { fn type_of() -> &'static str { stringify!(#ident) } @@ -74,6 +80,7 @@ pub fn decode_error(ident: &Ident) -> proc_macro2::TokenStream { pub fn print_program_error( ident: &Ident, variants: &Punctuated, + solana_program_crate: &Path, ) -> proc_macro2::TokenStream { let ppe_match_arms = variants.iter().map(|variant| { let variant_ident = &variant.ident; @@ -81,18 +88,18 @@ pub fn print_program_error( .unwrap_or_else(|| String::from("Unknown custom program error")); quote! { #ident::#variant_ident => { - ::solana_program::msg!(#error_msg) + #solana_program_crate::msg!(#error_msg) } } }); quote! { - impl ::solana_program::program_error::PrintProgramError for #ident { + impl #solana_program_crate::program_error::PrintProgramError for #ident { fn print(&self) where E: 'static + std::error::Error - + ::solana_program::decode_error::DecodeError - + ::solana_program::program_error::PrintProgramError + + #solana_program_crate::decode_error::DecodeError + + #solana_program_crate::program_error::PrintProgramError + num_traits::FromPrimitive, { match self { @@ -128,9 +135,9 @@ pub fn spl_program_error( let ident = &item_enum.ident; let variants = &item_enum.variants; - let into_program_error = into_program_error(ident); - let decode_error = decode_error(ident); - let print_program_error = print_program_error(ident, variants); + let into_program_error = into_program_error(ident, &args.solana_program_crate); + let decode_error = decode_error(ident, &args.solana_program_crate); + let print_program_error = print_program_error(ident, variants, &args.solana_program_crate); quote! { #[repr(u32)] diff --git a/libraries/program-error/derive/src/parser.rs b/libraries/program-error/derive/src/parser.rs index a09ffc7ba7c..8228b50c6f7 100644 --- a/libraries/program-error/derive/src/parser.rs +++ b/libraries/program-error/derive/src/parser.rs @@ -1,11 +1,11 @@ //! Token parsing use { - proc_macro2::Ident, + proc_macro2::{Ident, Span}, syn::{ parse::{Parse, ParseStream}, token::Comma, - LitInt, Token, + LitInt, LitStr, Path, Token, }, }; @@ -14,20 +14,31 @@ pub struct SplProgramErrorArgs { /// Whether to hash the error codes using `solana_program::hash` /// or to use the default error code assigned by `num_traits`. pub hash_error_code_start: Option, + /// Crate to use for solana_program + pub solana_program_crate: Path, } impl Parse for SplProgramErrorArgs { fn parse(input: ParseStream) -> syn::Result { - if input.is_empty() { - return Ok(Self { - hash_error_code_start: None, - }); - } - match SplProgramErrorArgParser::parse(input)? { - SplProgramErrorArgParser::HashErrorCodes { value, .. } => Ok(Self { - hash_error_code_start: Some(value.base10_parse::()?), - }), + let default_solana_program_crate = Ident::new("solana_program", Span::call_site()); + let mut hash_error_code_start = None; + let mut solana_program_crate = None; + while !input.is_empty() { + match SplProgramErrorArgParser::parse(input)? { + SplProgramErrorArgParser::HashErrorCodes { value, .. } => { + hash_error_code_start = Some(value.base10_parse::()?); + } + SplProgramErrorArgParser::SolanaProgramCrate { value, .. } => { + solana_program_crate = value.parse()?; + } + } } + Ok(Self { + hash_error_code_start, + solana_program_crate: solana_program_crate + .unwrap_or(default_solana_program_crate) + .into(), + }) } } @@ -35,30 +46,45 @@ impl Parse for SplProgramErrorArgs { /// ie. `#[spl_program_error(hash_error_code_start = 1275525928)]` enum SplProgramErrorArgParser { HashErrorCodes { - _ident: Ident, _equals_sign: Token![=], value: LitInt, _comma: Option, }, + SolanaProgramCrate { + _equals_sign: Token![=], + value: LitStr, + _comma: Option, + }, } impl Parse for SplProgramErrorArgParser { fn parse(input: ParseStream) -> syn::Result { - let _ident = { - let ident = input.parse::()?; - if ident != "hash_error_code_start" { - return Err(input.error("Expected argument 'hash_error_code_start'")); + let ident = input.parse::()?; + match ident.to_string().as_str() { + "hash_error_code_start" => { + let _equals_sign = input.parse::()?; + let value = input.parse::()?; + let _comma: Option = input.parse().unwrap_or(None); + Ok(Self::HashErrorCodes { + _equals_sign, + value, + _comma, + }) } - ident - }; - let _equals_sign = input.parse::()?; - let value = input.parse::()?; - let _comma: Option = input.parse().unwrap_or(None); - Ok(Self::HashErrorCodes { - _ident, - _equals_sign, - value, - _comma, - }) + "solana_program_crate" => { + let _equals_sign = input.parse::()?; + let value = input.parse::()?; + let _comma: Option = input.parse().unwrap_or(None); + Ok(Self::SolanaProgramCrate { + _equals_sign, + value, + _comma, + }) + } + _ => { + Err(input + .error("Expected argument 'hash_error_code_start' or 'solana_program_crate'")) + } + } } } diff --git a/libraries/program-error/tests/spl.rs b/libraries/program-error/tests/spl.rs index bbdb9af0aca..cc97c8d2e0e 100644 --- a/libraries/program-error/tests/spl.rs +++ b/libraries/program-error/tests/spl.rs @@ -72,3 +72,20 @@ fn test_library_error_codes() { first_error_as_u32 + 3, ); } + +/// Example error with solana_program crate set +#[spl_program_error(solana_program_crate = "solana_program")] +enum ExampleSolanaProgramCrateError { + /// This is a very informative error + #[error("This is a very informative error")] + VeryInformativeError, + /// This is a super important error + #[error("This is a super important error")] + SuperImportantError, +} + +/// Tests that all macros compile +#[test] +fn test_macros_compile_with_solana_program_crate() { + let _ = ExampleSolanaProgramCrateError::VeryInformativeError; +} diff --git a/libraries/type-length-value/src/error.rs b/libraries/type-length-value/src/error.rs index 48e5d9efa34..420a3603604 100644 --- a/libraries/type-length-value/src/error.rs +++ b/libraries/type-length-value/src/error.rs @@ -3,7 +3,10 @@ use spl_program_error::*; /// Errors that may be returned by the Token program. -#[spl_program_error(hash_error_code_start = 1_202_666_432)] +#[spl_program_error( + hash_error_code_start = 1_202_666_432, + solana_program_crate = "solana_program" +)] pub enum TlvError { /// Type not found in TLV data #[error("Type not found in TLV data")] From 51f5807f1c84988dc92c34ccaa3aabc5a6a6bb2a Mon Sep 17 00:00:00 2001 From: Jon C Date: Tue, 13 Aug 2024 00:08:10 +0200 Subject: [PATCH 2/4] Address feedback --- .../program-error/derive/src/macro_impl.rs | 48 +++++++----- libraries/program-error/derive/src/parser.rs | 76 ++++++++++++++++--- 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/libraries/program-error/derive/src/macro_impl.rs b/libraries/program-error/derive/src/macro_impl.rs index 172fc6ecc18..c70bf3227af 100644 --- a/libraries/program-error/derive/src/macro_impl.rs +++ b/libraries/program-error/derive/src/macro_impl.rs @@ -1,13 +1,13 @@ //! The actual token generator for the macro use { - crate::parser::SplProgramErrorArgs, + crate::parser::{SolanaProgram, SplProgramErrorArgs}, proc_macro2::Span, quote::quote, sha2::{Digest, Sha256}, syn::{ punctuated::Punctuated, token::Comma, Expr, ExprLit, Ident, ItemEnum, Lit, LitInt, LitStr, - Path, Token, Variant, + Token, Variant, }, }; @@ -36,15 +36,12 @@ pub enum MacroType { impl MacroType { /// Generates the corresponding tokens based on variant selection pub fn generate_tokens(&mut self) -> proc_macro2::TokenStream { - let default_solana_program_crate_path = - Ident::new("solana_program", Span::call_site()).into(); + let default_solana_program = SolanaProgram::default(); match self { - Self::IntoProgramError { ident } => { - into_program_error(ident, &default_solana_program_crate_path) - } - Self::DecodeError { ident } => decode_error(ident, &default_solana_program_crate_path), + Self::IntoProgramError { ident } => into_program_error(ident, &default_solana_program), + Self::DecodeError { ident } => decode_error(ident, &default_solana_program), Self::PrintProgramError { ident, variants } => { - print_program_error(ident, variants, &default_solana_program_crate_path) + print_program_error(ident, variants, &default_solana_program) } Self::SplProgramError { args, item_enum } => spl_program_error(args, item_enum), } @@ -54,25 +51,33 @@ impl MacroType { /// Builds the implementation of /// `Into` More specifically, /// implements `From for solana_program::program_error::ProgramError` -pub fn into_program_error(ident: &Ident, solana_program_crate: &Path) -> proc_macro2::TokenStream { - quote! { +pub fn into_program_error( + ident: &Ident, + solana_program_crate: &SolanaProgram, +) -> proc_macro2::TokenStream { + let this_impl = quote! { impl From<#ident> for #solana_program_crate::program_error::ProgramError { fn from(e: #ident) -> Self { #solana_program_crate::program_error::ProgramError::Custom(e as u32) } } - } + }; + solana_program_crate.wrap(this_impl) } /// Builds the implementation of `solana_program::decode_error::DecodeError` -pub fn decode_error(ident: &Ident, solana_program_crate: &Path) -> proc_macro2::TokenStream { - quote! { +pub fn decode_error( + ident: &Ident, + solana_program_crate: &SolanaProgram, +) -> proc_macro2::TokenStream { + let this_impl = quote! { impl #solana_program_crate::decode_error::DecodeError for #ident { fn type_of() -> &'static str { stringify!(#ident) } } - } + }; + solana_program_crate.wrap(this_impl) } /// Builds the implementation of @@ -80,7 +85,7 @@ pub fn decode_error(ident: &Ident, solana_program_crate: &Path) -> proc_macro2:: pub fn print_program_error( ident: &Ident, variants: &Punctuated, - solana_program_crate: &Path, + solana_program_crate: &SolanaProgram, ) -> proc_macro2::TokenStream { let ppe_match_arms = variants.iter().map(|variant| { let variant_ident = &variant.ident; @@ -92,7 +97,7 @@ pub fn print_program_error( } } }); - quote! { + let this_impl = quote! { impl #solana_program_crate::program_error::PrintProgramError for #ident { fn print(&self) where @@ -107,7 +112,8 @@ pub fn print_program_error( } } } - } + }; + solana_program_crate.wrap(this_impl) } /// Helper to parse out the string literal from the `#[error(..)]` attribute @@ -135,9 +141,9 @@ pub fn spl_program_error( let ident = &item_enum.ident; let variants = &item_enum.variants; - let into_program_error = into_program_error(ident, &args.solana_program_crate); - let decode_error = decode_error(ident, &args.solana_program_crate); - let print_program_error = print_program_error(ident, variants, &args.solana_program_crate); + let into_program_error = into_program_error(ident, &args.import); + let decode_error = decode_error(ident, &args.import); + let print_program_error = print_program_error(ident, variants, &args.import); quote! { #[repr(u32)] diff --git a/libraries/program-error/derive/src/parser.rs b/libraries/program-error/derive/src/parser.rs index 8228b50c6f7..e84e276de09 100644 --- a/libraries/program-error/derive/src/parser.rs +++ b/libraries/program-error/derive/src/parser.rs @@ -1,11 +1,12 @@ //! Token parsing use { - proc_macro2::{Ident, Span}, + proc_macro2::{Ident, Span, TokenStream}, + quote::quote, syn::{ parse::{Parse, ParseStream}, token::Comma, - LitInt, LitStr, Path, Token, + LitInt, LitStr, Token, }, }; @@ -15,29 +16,58 @@ pub struct SplProgramErrorArgs { /// or to use the default error code assigned by `num_traits`. pub hash_error_code_start: Option, /// Crate to use for solana_program - pub solana_program_crate: Path, + pub import: SolanaProgram, +} + +/// Struct representing the path to a `solana_program` crate, which may be +/// renamed or otherwise. +pub struct SolanaProgram { + import: Ident, + explicit: bool, +} +impl quote::ToTokens for SolanaProgram { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.import.to_tokens(tokens); + } +} +impl SolanaProgram { + pub fn wrap(&self, output: TokenStream) -> TokenStream { + if self.explicit { + output + } else { + anon_const_trick(output) + } + } +} +impl Default for SolanaProgram { + fn default() -> Self { + Self { + import: Ident::new("_solana_program", Span::call_site()), + explicit: false, + } + } } impl Parse for SplProgramErrorArgs { fn parse(input: ParseStream) -> syn::Result { - let default_solana_program_crate = Ident::new("solana_program", Span::call_site()); let mut hash_error_code_start = None; - let mut solana_program_crate = None; + let mut import = None; while !input.is_empty() { match SplProgramErrorArgParser::parse(input)? { SplProgramErrorArgParser::HashErrorCodes { value, .. } => { hash_error_code_start = Some(value.base10_parse::()?); } SplProgramErrorArgParser::SolanaProgramCrate { value, .. } => { - solana_program_crate = value.parse()?; + import = Some(SolanaProgram { + import: value.parse()?, + explicit: true, + }); } } } Ok(Self { hash_error_code_start, - solana_program_crate: solana_program_crate - .unwrap_or(default_solana_program_crate) - .into(), + import: import.unwrap_or(SolanaProgram::default()), }) } } @@ -88,3 +118,31 @@ impl Parse for SplProgramErrorArgParser { } } } + +// Within `exp`, you can bring things into scope with `extern crate`. +// +// We don't want to assume that `solana_program::` is in scope - the user may +// have imported it under a different name, or may have imported it in a +// non-toplevel module (common when putting impls behind a feature gate). +// +// Solution: let's just generate `extern crate solana_program as +// _solana_program` and then refer to `_solana_program` in the derived code. +// However, macros are not allowed to produce `extern crate` statements at the +// toplevel. +// +// Solution: let's generate `mod _impl_foo` and import solana_program within +// that. However, now we lose access to private members of the surrounding +// module. This is a problem if, for example, we're deriving for a newtype, +// where the inner type is defined in the same module, but not exported. +// +// Solution: use the anonymous const trick. For some reason, `extern crate` +// statements are allowed here, but everything from the surrounding module is in +// scope. This trick is taken from serde and num_traits. +fn anon_const_trick(exp: TokenStream) -> TokenStream { + quote! { + const _: () = { + extern crate solana_program as _solana_program; + #exp + }; + } +} From c28ef0e3ebcf75cb3b51c1b20af2df044aaa2ada Mon Sep 17 00:00:00 2001 From: Jon C Date: Tue, 13 Aug 2024 00:22:58 +0200 Subject: [PATCH 3/4] Rename solana_program_crate -> solana_program --- .../program-error/derive/src/macro_impl.rs | 32 ++++++++----------- libraries/program-error/derive/src/parser.rs | 7 ++-- libraries/program-error/tests/spl.rs | 2 +- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/libraries/program-error/derive/src/macro_impl.rs b/libraries/program-error/derive/src/macro_impl.rs index c70bf3227af..1f90cd55613 100644 --- a/libraries/program-error/derive/src/macro_impl.rs +++ b/libraries/program-error/derive/src/macro_impl.rs @@ -51,33 +51,27 @@ impl MacroType { /// Builds the implementation of /// `Into` More specifically, /// implements `From for solana_program::program_error::ProgramError` -pub fn into_program_error( - ident: &Ident, - solana_program_crate: &SolanaProgram, -) -> proc_macro2::TokenStream { +pub fn into_program_error(ident: &Ident, import: &SolanaProgram) -> proc_macro2::TokenStream { let this_impl = quote! { - impl From<#ident> for #solana_program_crate::program_error::ProgramError { + impl From<#ident> for #import::program_error::ProgramError { fn from(e: #ident) -> Self { - #solana_program_crate::program_error::ProgramError::Custom(e as u32) + #import::program_error::ProgramError::Custom(e as u32) } } }; - solana_program_crate.wrap(this_impl) + import.wrap(this_impl) } /// Builds the implementation of `solana_program::decode_error::DecodeError` -pub fn decode_error( - ident: &Ident, - solana_program_crate: &SolanaProgram, -) -> proc_macro2::TokenStream { +pub fn decode_error(ident: &Ident, import: &SolanaProgram) -> proc_macro2::TokenStream { let this_impl = quote! { - impl #solana_program_crate::decode_error::DecodeError for #ident { + impl #import::decode_error::DecodeError for #ident { fn type_of() -> &'static str { stringify!(#ident) } } }; - solana_program_crate.wrap(this_impl) + import.wrap(this_impl) } /// Builds the implementation of @@ -85,7 +79,7 @@ pub fn decode_error( pub fn print_program_error( ident: &Ident, variants: &Punctuated, - solana_program_crate: &SolanaProgram, + import: &SolanaProgram, ) -> proc_macro2::TokenStream { let ppe_match_arms = variants.iter().map(|variant| { let variant_ident = &variant.ident; @@ -93,18 +87,18 @@ pub fn print_program_error( .unwrap_or_else(|| String::from("Unknown custom program error")); quote! { #ident::#variant_ident => { - #solana_program_crate::msg!(#error_msg) + #import::msg!(#error_msg) } } }); let this_impl = quote! { - impl #solana_program_crate::program_error::PrintProgramError for #ident { + impl #import::program_error::PrintProgramError for #ident { fn print(&self) where E: 'static + std::error::Error - + #solana_program_crate::decode_error::DecodeError - + #solana_program_crate::program_error::PrintProgramError + + #import::decode_error::DecodeError + + #import::program_error::PrintProgramError + num_traits::FromPrimitive, { match self { @@ -113,7 +107,7 @@ pub fn print_program_error( } } }; - solana_program_crate.wrap(this_impl) + import.wrap(this_impl) } /// Helper to parse out the string literal from the `#[error(..)]` attribute diff --git a/libraries/program-error/derive/src/parser.rs b/libraries/program-error/derive/src/parser.rs index e84e276de09..7e01cb728cc 100644 --- a/libraries/program-error/derive/src/parser.rs +++ b/libraries/program-error/derive/src/parser.rs @@ -101,7 +101,7 @@ impl Parse for SplProgramErrorArgParser { _comma, }) } - "solana_program_crate" => { + "solana_program" => { let _equals_sign = input.parse::()?; let value = input.parse::()?; let _comma: Option = input.parse().unwrap_or(None); @@ -111,10 +111,7 @@ impl Parse for SplProgramErrorArgParser { _comma, }) } - _ => { - Err(input - .error("Expected argument 'hash_error_code_start' or 'solana_program_crate'")) - } + _ => Err(input.error("Expected argument 'hash_error_code_start' or 'solana_program'")), } } } diff --git a/libraries/program-error/tests/spl.rs b/libraries/program-error/tests/spl.rs index cc97c8d2e0e..d772d24f6c5 100644 --- a/libraries/program-error/tests/spl.rs +++ b/libraries/program-error/tests/spl.rs @@ -74,7 +74,7 @@ fn test_library_error_codes() { } /// Example error with solana_program crate set -#[spl_program_error(solana_program_crate = "solana_program")] +#[spl_program_error(solana_program = "solana_program")] enum ExampleSolanaProgramCrateError { /// This is a very informative error #[error("This is a very informative error")] From 97a032af155ea32272ff6332087ea8dc8f68563e Mon Sep 17 00:00:00 2001 From: Jon C Date: Tue, 13 Aug 2024 00:29:25 +0200 Subject: [PATCH 4/4] Oops, change the name everywhere --- libraries/type-length-value/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/type-length-value/src/error.rs b/libraries/type-length-value/src/error.rs index 420a3603604..583810e2b40 100644 --- a/libraries/type-length-value/src/error.rs +++ b/libraries/type-length-value/src/error.rs @@ -5,7 +5,7 @@ use spl_program_error::*; /// Errors that may be returned by the Token program. #[spl_program_error( hash_error_code_start = 1_202_666_432, - solana_program_crate = "solana_program" + solana_program = "solana_program" )] pub enum TlvError { /// Type not found in TLV data