Skip to content
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

Remove fixed workaround in impl_runtime_apis #5839

Merged
merged 9 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use sc_cli::{Result, SubstrateCli};
use sp_runtime::traits::AccountIdConversion;
#[cfg(feature = "runtime-benchmarks")]
use sp_runtime::traits::HashingFor;
use std::panic::{RefUnwindSafe, UnwindSafe};

/// Structure that can be used in order to provide customizers for different functionalities of the
/// node binary that is being built using this library.
Expand All @@ -55,8 +54,7 @@ pub fn new_aura_node_spec<Block>(
extra_args: &NodeExtraArgs,
) -> Box<dyn DynNodeSpec>
where
Block: NodeBlock + UnwindSafe + RefUnwindSafe,
Block::BoundedHeader: UnwindSafe + RefUnwindSafe,
Block: NodeBlock,
{
match aura_id {
AuraConsensusId::Sr25519 => crate::service::new_aura_node_spec::<
Expand Down
21 changes: 21 additions & 0 deletions prdoc/pr_5839.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Remove internal workaround for compiler bug

doc:
- audience:
- Runtime Dev
- Node Dev
description: |
Remove a workaround we had in the `impl_runtime_apis` macro for a compiler bug that has been long fixed.
No impact on downstream users is expected, except relaxed trait bounds in a few places where the compiler
is now able to deduce more type info itself.

crates:
- name: sp-api-proc-macro
bump: patch
- name: frame-support-procedural
bump: patch
- name: polkadot-parachain-lib
bump: patch
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub fn expand(def: Def, legacy_ordering: bool) -> TokenStream2 {
};

let res = expander::Expander::new("construct_runtime")
.dry(std::env::var("FRAME_EXPAND").is_err())
.dry(std::env::var("EXPAND_MACROS").is_err())
.verbose(true)
.write_to_out_dir(res)
.expect("Does not fail because of IO in OUT_DIR; qed");
Expand Down
22 changes: 3 additions & 19 deletions substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::{
common::API_VERSION_ATTRIBUTE,
utils::{
extract_all_signature_types, extract_block_type_from_trait_path, extract_impl_trait,
extract_block_type_from_trait_path, extract_impl_trait,
extract_parameter_names_types_and_borrows, generate_crate_access,
generate_runtime_mod_name_for_trait, parse_runtime_api_version, prefix_function_with_trait,
versioned_trait_name, AllowSelfRefInParameters, RequireQualifiedTraitPath,
Expand Down Expand Up @@ -632,21 +632,16 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> {
}

fn fold_item_impl(&mut self, mut input: ItemImpl) -> ItemImpl {
// All this `UnwindSafe` magic below here is required for this rust bug:
// https://github.com/rust-lang/rust/issues/24159
// Before we directly had the final block type and rust could determine that it is unwind
// safe, but now we just have a generic parameter `Block`.

let crate_ = generate_crate_access();

// Implement the trait for the `RuntimeApiImpl`
input.self_ty =
Box::new(parse_quote!( RuntimeApiImpl<__SrApiBlock__, RuntimeApiImplCall> ));

input.generics.params.push(parse_quote!(
__SrApiBlock__: #crate_::BlockT + std::panic::UnwindSafe +
std::panic::RefUnwindSafe
__SrApiBlock__: #crate_::BlockT
));

input
.generics
.params
Expand All @@ -661,17 +656,6 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> {

where_clause.predicates.push(parse_quote! { &'static RuntimeApiImplCall: Send });

// Require that all types used in the function signatures are unwind safe.
extract_all_signature_types(&input.items).iter().for_each(|i| {
where_clause.predicates.push(parse_quote! {
#i: std::panic::UnwindSafe + std::panic::RefUnwindSafe
});
});

where_clause.predicates.push(parse_quote! {
__SrApiBlock__::Header: std::panic::UnwindSafe + std::panic::RefUnwindSafe
});

input.attrs = filter_cfg_attrs(&input.attrs);

fold::fold_item_impl(self, input)
Expand Down
35 changes: 2 additions & 33 deletions substrate/primitives/api/proc-macro/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use proc_macro_crate::{crate_name, FoundCrate};
use quote::{format_ident, quote};
use syn::{
parse_quote, punctuated::Punctuated, spanned::Spanned, token::And, Attribute, Error, Expr,
ExprLit, FnArg, GenericArgument, Ident, ImplItem, ItemImpl, Lit, Meta, MetaNameValue, Pat,
Path, PathArguments, Result, ReturnType, Signature, Token, Type, TypePath,
ExprLit, FnArg, GenericArgument, Ident, ItemImpl, Lit, Meta, MetaNameValue, Pat, Path,
PathArguments, Result, ReturnType, Signature, Token, Type, TypePath,
};

/// Generates the access to the `sc_client` crate.
Expand Down Expand Up @@ -159,37 +159,6 @@ pub fn prefix_function_with_trait<F: ToString>(trait_: &Ident, function: &F) ->
format!("{}_{}", trait_, function.to_string())
}

/// Extract all types that appear in signatures in the given `ImplItem`'s.
///
/// If a type is a reference, the inner type is extracted (without the reference).
pub fn extract_all_signature_types(items: &[ImplItem]) -> Vec<Type> {
items
.iter()
.filter_map(|i| match i {
ImplItem::Fn(method) => Some(&method.sig),
_ => None,
})
.flat_map(|sig| {
let ret_ty = match &sig.output {
ReturnType::Default => None,
ReturnType::Type(_, ty) => Some((**ty).clone()),
};

sig.inputs
.iter()
.filter_map(|i| match i {
FnArg::Typed(arg) => Some(&arg.ty),
_ => None,
})
.map(|ty| match &**ty {
Type::Reference(t) => (*t.elem).clone(),
_ => (**ty).clone(),
})
.chain(ret_ty)
})
.collect()
}

/// Extracts the block type from a trait path.
///
/// It is expected that the block type is the first type in the generic arguments.
Expand Down