Skip to content

Commit

Permalink
Remove fixed workaround in impl_runtime_apis (#5839)
Browse files Browse the repository at this point in the history
This PR removes a workaround which had a reference comment to a rust
compiler issue. The issue has been fixed and we should be able to remove
that trait bound.
  • Loading branch information
skunert committed Sep 27, 2024
1 parent 6d1943b commit a5e40d0
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 56 deletions.
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

0 comments on commit a5e40d0

Please sign in to comment.