From 230876c38a008e78ff4ec75b02c26ef0ae85bf04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 1 Jun 2023 15:29:32 +0100 Subject: [PATCH] sp-api: Make the generated code act based on `std` in `sp-api` (#14267) * sp-api: Make the generated code act based on `std` in `sp-api` Instead of letting the macro generate code that checks if the `std` feature is enabled, it will now generate code that checks if the `std` feature is enabled for the `sp-api` crate. The old implementation basically required that the crate in which the macro was used, had a `std` feature. Now we don't have this requirement anymore and act accordingly the feature in `sp-api` directly. * Missing feature! --------- Co-authored-by: parity-processbot <> --- frame/executive/Cargo.toml | 1 + .../api/proc-macro/src/decl_runtime_apis.rs | 21 +- .../api/proc-macro/src/impl_runtime_apis.rs | 344 +++++++++--------- primitives/api/src/lib.rs | 3 + primitives/api/test/Cargo.toml | 5 - primitives/core/src/lib.rs | 12 + 6 files changed, 200 insertions(+), 186 deletions(-) diff --git a/frame/executive/Cargo.toml b/frame/executive/Cargo.toml index dfc8e85afd8f2..2532df31682f7 100644 --- a/frame/executive/Cargo.toml +++ b/frame/executive/Cargo.toml @@ -42,6 +42,7 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "frame-try-runtime/std", "scale-info/std", "sp-core/std", "sp-io/std", diff --git a/primitives/api/proc-macro/src/decl_runtime_apis.rs b/primitives/api/proc-macro/src/decl_runtime_apis.rs index cde33c19016bd..ca2bd6b070a7e 100644 --- a/primitives/api/proc-macro/src/decl_runtime_apis.rs +++ b/primitives/api/proc-macro/src/decl_runtime_apis.rs @@ -539,8 +539,6 @@ impl<'a> Fold for ToClientSideDecl<'a> { input.supertraits.push(parse_quote!( #crate_::Core<#block_ident> )); } - // The client side trait is only required when compiling with the feature `std` or `test`. - input.attrs.push(parse_quote!( #[cfg(any(feature = "std", test))] )); input.items = self.fold_item_trait_items(input.items, input.generics.params.len()); fold::fold_item_trait(self, input) @@ -584,12 +582,13 @@ fn generate_runtime_info_impl(trait_: &ItemTrait, version: u64) -> TokenStream { }); quote!( - #[cfg(any(feature = "std", test))] - impl < #( #impl_generics, )* > #crate_::RuntimeApiInfo - for dyn #trait_name < #( #ty_generics, )* > - { - #id - #version + #crate_::std_enabled! { + impl < #( #impl_generics, )* > #crate_::RuntimeApiInfo + for dyn #trait_name < #( #ty_generics, )* > + { + #id + #version + } } ) } @@ -636,7 +635,11 @@ fn generate_client_side_decls(decls: &[ItemTrait]) -> Result { let runtime_info = api_version.map(|v| generate_runtime_info_impl(&decl, v))?; - result.push(quote!( #decl #runtime_info #( #errors )* )); + result.push(quote!( + #crate_::std_enabled! { #decl } + #runtime_info + #( #errors )* + )); } Ok(quote!( #( #result )* )) diff --git a/primitives/api/proc-macro/src/impl_runtime_apis.rs b/primitives/api/proc-macro/src/impl_runtime_apis.rs index b8dcf625df45e..fa933ceb91797 100644 --- a/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -172,11 +172,12 @@ fn generate_dispatch_function(impls: &[ItemImpl]) -> Result { }); Ok(quote!( - #[cfg(feature = "std")] - pub fn dispatch(method: &str, mut #data: &[u8]) -> Option> { - match method { - #( #impl_calls )* - _ => None, + #c::std_enabled! { + pub fn dispatch(method: &str, mut #data: &[u8]) -> Option> { + match method { + #( #impl_calls )* + _ => None, + } } } )) @@ -195,22 +196,23 @@ fn generate_wasm_interface(impls: &[ItemImpl]) -> Result { Ident::new(&prefix_function_with_trait(&trait_, &fn_name), Span::call_site()); quote!( - #( #attrs )* - #[cfg(not(feature = "std"))] - #[no_mangle] - pub unsafe fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 { - let mut #input = if input_len == 0 { - &[0u8; 0] - } else { - unsafe { - #c::slice::from_raw_parts(input_data, input_len) - } - }; - - #c::init_runtime_logger(); - - let output = (move || { #impl_ })(); - #c::to_substrate_wasm_fn_return_value(&output) + #c::std_disabled! { + #( #attrs )* + #[no_mangle] + pub unsafe fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 { + let mut #input = if input_len == 0 { + &[0u8; 0] + } else { + unsafe { + #c::slice::from_raw_parts(input_data, input_len) + } + }; + + #c::init_runtime_logger(); + + let output = (move || { #impl_ })(); + #c::to_substrate_wasm_fn_return_value(&output) + } } ) }); @@ -224,175 +226,173 @@ fn generate_runtime_api_base_structures() -> Result { Ok(quote!( pub struct RuntimeApi {} /// Implements all runtime apis for the client side. - #[cfg(any(feature = "std", test))] - pub struct RuntimeApiImpl + 'static> { - call: &'static C, - commit_on_success: std::cell::RefCell, - changes: std::cell::RefCell<#crate_::OverlayedChanges>, - storage_transaction_cache: std::cell::RefCell< - #crate_::StorageTransactionCache - >, - recorder: std::option::Option<#crate_::ProofRecorder>, - } + #crate_::std_enabled! { + pub struct RuntimeApiImpl + 'static> { + call: &'static C, + commit_on_success: std::cell::RefCell, + changes: std::cell::RefCell<#crate_::OverlayedChanges>, + storage_transaction_cache: std::cell::RefCell< + #crate_::StorageTransactionCache + >, + recorder: std::option::Option<#crate_::ProofRecorder>, + } - #[cfg(any(feature = "std", test))] - impl> #crate_::ApiExt for - RuntimeApiImpl - { - type StateBackend = C::StateBackend; + impl> #crate_::ApiExt for + RuntimeApiImpl + { + type StateBackend = C::StateBackend; - fn execute_in_transaction #crate_::TransactionOutcome, R>( - &self, - call: F, - ) -> R where Self: Sized { - self.start_transaction(); + fn execute_in_transaction #crate_::TransactionOutcome, R>( + &self, + call: F, + ) -> R where Self: Sized { + self.start_transaction(); - *std::cell::RefCell::borrow_mut(&self.commit_on_success) = false; - let res = call(self); - *std::cell::RefCell::borrow_mut(&self.commit_on_success) = true; + *std::cell::RefCell::borrow_mut(&self.commit_on_success) = false; + let res = call(self); + *std::cell::RefCell::borrow_mut(&self.commit_on_success) = true; - self.commit_or_rollback(std::matches!(res, #crate_::TransactionOutcome::Commit(_))); + self.commit_or_rollback(std::matches!(res, #crate_::TransactionOutcome::Commit(_))); - res.into_inner() - } + res.into_inner() + } - fn has_api( - &self, - at: ::Hash, - ) -> std::result::Result where Self: Sized { - #crate_::CallApiAt::::runtime_version_at(self.call, at) + fn has_api( + &self, + at: ::Hash, + ) -> std::result::Result where Self: Sized { + #crate_::CallApiAt::::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::has_api_with(&v, &A::ID, |v| v == A::VERSION)) - } + } - fn has_api_with bool>( - &self, - at: ::Hash, - pred: P, - ) -> std::result::Result where Self: Sized { - #crate_::CallApiAt::::runtime_version_at(self.call, at) + fn has_api_with bool>( + &self, + at: ::Hash, + pred: P, + ) -> std::result::Result where Self: Sized { + #crate_::CallApiAt::::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::has_api_with(&v, &A::ID, pred)) - } + } - fn api_version( - &self, - at: ::Hash, - ) -> std::result::Result, #crate_::ApiError> where Self: Sized { - #crate_::CallApiAt::::runtime_version_at(self.call, at) + fn api_version( + &self, + at: ::Hash, + ) -> std::result::Result, #crate_::ApiError> where Self: Sized { + #crate_::CallApiAt::::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::api_version(&v, &A::ID)) - } + } - fn record_proof(&mut self) { - self.recorder = std::option::Option::Some(std::default::Default::default()); - } + fn record_proof(&mut self) { + self.recorder = std::option::Option::Some(std::default::Default::default()); + } - fn proof_recorder(&self) -> std::option::Option<#crate_::ProofRecorder> { - std::clone::Clone::clone(&self.recorder) - } + fn proof_recorder(&self) -> std::option::Option<#crate_::ProofRecorder> { + std::clone::Clone::clone(&self.recorder) + } - fn extract_proof( - &mut self, - ) -> std::option::Option<#crate_::StorageProof> { - let recorder = std::option::Option::take(&mut self.recorder); - std::option::Option::map(recorder, |recorder| { - #crate_::ProofRecorder::::drain_storage_proof(recorder) - }) - } + fn extract_proof( + &mut self, + ) -> std::option::Option<#crate_::StorageProof> { + let recorder = std::option::Option::take(&mut self.recorder); + std::option::Option::map(recorder, |recorder| { + #crate_::ProofRecorder::::drain_storage_proof(recorder) + }) + } - fn into_storage_changes( - &self, - backend: &Self::StateBackend, - parent_hash: Block::Hash, - ) -> core::result::Result< - #crate_::StorageChanges, + fn into_storage_changes( + &self, + backend: &Self::StateBackend, + parent_hash: Block::Hash, + ) -> core::result::Result< + #crate_::StorageChanges, String - > where Self: Sized { - let state_version = #crate_::CallApiAt::::runtime_version_at(self.call, std::clone::Clone::clone(&parent_hash)) - .map(|v| #crate_::RuntimeVersion::state_version(&v)) - .map_err(|e| format!("Failed to get state version: {}", e))?; - - #crate_::OverlayedChanges::into_storage_changes( - std::cell::RefCell::take(&self.changes), - backend, - core::cell::RefCell::take(&self.storage_transaction_cache), - state_version, - ) + > where Self: Sized { + let state_version = #crate_::CallApiAt::::runtime_version_at(self.call, std::clone::Clone::clone(&parent_hash)) + .map(|v| #crate_::RuntimeVersion::state_version(&v)) + .map_err(|e| format!("Failed to get state version: {}", e))?; + + #crate_::OverlayedChanges::into_storage_changes( + std::cell::RefCell::take(&self.changes), + backend, + core::cell::RefCell::take(&self.storage_transaction_cache), + state_version, + ) + } } - } - #[cfg(any(feature = "std", test))] - impl #crate_::ConstructRuntimeApi - for RuntimeApi - where - C: #crate_::CallApiAt + 'static, - { - type RuntimeApi = RuntimeApiImpl; - - fn construct_runtime_api<'a>( - call: &'a C, - ) -> #crate_::ApiRef<'a, Self::RuntimeApi> { - RuntimeApiImpl { - call: unsafe { std::mem::transmute(call) }, - commit_on_success: true.into(), - changes: std::default::Default::default(), - recorder: std::default::Default::default(), - storage_transaction_cache: std::default::Default::default(), - }.into() + impl #crate_::ConstructRuntimeApi + for RuntimeApi + where + C: #crate_::CallApiAt + 'static, + { + type RuntimeApi = RuntimeApiImpl; + + fn construct_runtime_api<'a>( + call: &'a C, + ) -> #crate_::ApiRef<'a, Self::RuntimeApi> { + RuntimeApiImpl { + call: unsafe { std::mem::transmute(call) }, + commit_on_success: true.into(), + changes: std::default::Default::default(), + recorder: std::default::Default::default(), + storage_transaction_cache: std::default::Default::default(), + }.into() + } } - } - #[cfg(any(feature = "std", test))] - impl> RuntimeApiImpl { - fn commit_or_rollback(&self, commit: bool) { - let proof = "\ + impl> RuntimeApiImpl { + fn commit_or_rollback(&self, commit: bool) { + let proof = "\ We only close a transaction when we opened one ourself. Other parts of the runtime that make use of transactions (state-machine) also balance their transactions. The runtime cannot close client initiated transactions; qed"; - if *std::cell::RefCell::borrow(&self.commit_on_success) { - let res = if commit { - let res = if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::::commit_transaction(&recorder) + if *std::cell::RefCell::borrow(&self.commit_on_success) { + let res = if commit { + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::commit_transaction(&recorder) + } else { + Ok(()) + }; + + let res2 = #crate_::OverlayedChanges::commit_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) } else { - Ok(()) + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::rollback_transaction(&recorder) + } else { + Ok(()) + }; + + let res2 = #crate_::OverlayedChanges::rollback_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) }; - let res2 = #crate_::OverlayedChanges::commit_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - - // Will panic on an `Err` below, however we should call commit - // on the recorder and the changes together. - std::result::Result::and(res, std::result::Result::map_err(res2, drop)) - } else { - let res = if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::::rollback_transaction(&recorder) - } else { - Ok(()) - }; - - let res2 = #crate_::OverlayedChanges::rollback_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - - // Will panic on an `Err` below, however we should call commit - // on the recorder and the changes together. - std::result::Result::and(res, std::result::Result::map_err(res2, drop)) - }; - - std::result::Result::expect(res, proof); + std::result::Result::expect(res, proof); + } } - } - fn start_transaction(&self) { - if !*std::cell::RefCell::borrow(&self.commit_on_success) { - return - } + fn start_transaction(&self) { + if !*std::cell::RefCell::borrow(&self.commit_on_success) { + return + } - #crate_::OverlayedChanges::start_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::::start_transaction(&recorder); + #crate_::OverlayedChanges::start_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::::start_transaction(&recorder); + } } } } @@ -571,10 +571,6 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> { input.attrs = filter_cfg_attrs(&input.attrs); - // The implementation for the `RuntimeApiImpl` is only required when compiling with - // the feature `std` or `test`. - input.attrs.push(parse_quote!( #[cfg(any(feature = "std", test))] )); - fold::fold_item_impl(self, input) } } @@ -595,7 +591,10 @@ fn generate_api_impl_for_runtime_api(impls: &[ItemImpl]) -> Result result.push(processed_impl); } - Ok(quote!( #( #result )* )) + + let crate_ = generate_crate_access(); + + Ok(quote!( #crate_::std_enabled! { #( #result )* } )) } fn populate_runtime_api_versions( @@ -612,13 +611,14 @@ fn populate_runtime_api_versions( )); sections.push(quote!( - #( #attrs )* - const _: () = { - // All sections with the same name are going to be merged by concatenation. - #[cfg(not(feature = "std"))] - #[link_section = "runtime_apis"] - static SECTION_CONTENTS: [u8; 12] = #crate_access::serialize_runtime_api_info(#id, #version); - }; + #crate_access::std_disabled! { + #( #attrs )* + const _: () = { + // All sections with the same name are going to be merged by concatenation. + #[link_section = "runtime_apis"] + static SECTION_CONTENTS: [u8; 12] = #crate_access::serialize_runtime_api_info(#id, #version); + }; + } )); } diff --git a/primitives/api/src/lib.rs b/primitives/api/src/lib.rs index 02770280f7b90..4e474e02deadc 100644 --- a/primitives/api/src/lib.rs +++ b/primitives/api/src/lib.rs @@ -750,3 +750,6 @@ decl_runtime_apis! { fn metadata_versions() -> sp_std::vec::Vec; } } + +sp_core::generate_feature_enabled_macro!(std_enabled, feature = "std", $); +sp_core::generate_feature_enabled_macro!(std_disabled, not(feature = "std"), $); diff --git a/primitives/api/test/Cargo.toml b/primitives/api/test/Cargo.toml index 7f5c527b1ba75..376375bcc1137 100644 --- a/primitives/api/test/Cargo.toml +++ b/primitives/api/test/Cargo.toml @@ -34,8 +34,3 @@ sp-core = { version = "21.0.0", path = "../../core" } [[bench]] name = "bench" harness = false - -# We only need this to generate the correct code. -[features] -default = [ "std" ] -std = [] diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index b61009bc640ee..951b481253b4e 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -442,6 +442,18 @@ pub const MAX_POSSIBLE_ALLOCATION: u32 = 33554432; // 2^25 bytes, 32 MiB /// /// These feature checking macros can be used to conditionally enable/disable code in a dependent /// crate based on a feature in the crate where the macro is called. +/// +/// # Example +///``` +/// sp_core::generate_feature_enabled_macro!(check_std_is_enabled, feature = "std", $); +/// sp_core::generate_feature_enabled_macro!(check_std_or_serde_is_enabled, any(feature = "std", feature = "serde"), $); +/// +/// // All the code passed to the macro will then conditionally compiled based on the features +/// // activated for the crate where the macro was generated. +/// check_std_is_enabled! { +/// struct StdEnabled; +/// } +///``` #[macro_export] // We need to skip formatting this macro because of this bug: // https://github.com/rust-lang/rustfmt/issues/5283