From 1c72d7137479d5ff500f4d76ff10020ca048779c Mon Sep 17 00:00:00 2001 From: Julian Eager Date: Sun, 13 Oct 2024 05:20:04 +0800 Subject: [PATCH] Fix storage expansion in pallet section (#6023) fixes #5320 @sam0x17 @gupnik # Description The issue could be confirmed with the added example. The cause is for macro hygiene, `entries` in the `#( #entries_builder )*` expansion won't be able to reference the `entries` defined outside. The solution here is to allow the reference to be passed into the expansion with closure. Or we could just switch to the unhygienic span with `quote::quote!` instead such that `entries` will resolve to the "outer" definition. --- prdoc/pr_6023.prdoc | 11 +++++ .../procedural/src/pallet/expand/storage.rs | 22 +++++---- .../test/tests/split_ui/pass/split_storage.rs | 49 +++++++++++++++++++ .../test/tests/split_ui/pass/storage/mod.rs | 27 ++++++++++ 4 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 prdoc/pr_6023.prdoc create mode 100644 substrate/frame/support/test/tests/split_ui/pass/split_storage.rs create mode 100644 substrate/frame/support/test/tests/split_ui/pass/storage/mod.rs diff --git a/prdoc/pr_6023.prdoc b/prdoc/pr_6023.prdoc new file mode 100644 index 0000000000000..3b3b5a4cb5fd3 --- /dev/null +++ b/prdoc/pr_6023.prdoc @@ -0,0 +1,11 @@ +title: Fix storage in pallet section + +doc: + - audience: Runtime Dev + description: | + Fix compilation of `pallet::storage` in a pallet section: a local binding definition was not + correctly referenced due to macro hygiene. + +crates: + - name: frame-support-procedural + bump: patch diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index e5bfa2793cbb2..10e674c3cb194 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -427,15 +427,17 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { }; entries_builder.push(quote::quote_spanned!(storage.attr_span => #(#cfg_attrs)* - { - <#full_ident as #frame_support::storage::StorageEntryMetadataBuilder>::build_metadata( - #deprecation, - #frame_support::__private::vec![ - #( #docs, )* - ], - &mut entries, - ); - } + (|entries: &mut #frame_support::__private::Vec<_>| { + { + <#full_ident as #frame_support::storage::StorageEntryMetadataBuilder>::build_metadata( + #deprecation, + #frame_support::__private::vec![ + #( #docs, )* + ], + entries, + ); + } + }) )) } @@ -911,7 +913,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { entries: { #[allow(unused_mut)] let mut entries = #frame_support::__private::vec![]; - #( #entries_builder )* + #( #entries_builder(&mut entries); )* entries }, } diff --git a/substrate/frame/support/test/tests/split_ui/pass/split_storage.rs b/substrate/frame/support/test/tests/split_ui/pass/split_storage.rs new file mode 100644 index 0000000000000..e8601587fac73 --- /dev/null +++ b/substrate/frame/support/test/tests/split_ui/pass/split_storage.rs @@ -0,0 +1,49 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use frame_support::pallet_macros::import_section; + +mod storage; + +#[import_section(storage::storage)] +#[frame_support::pallet(dev_mode)] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + const STORAGE_VERSION: StorageVersion = StorageVersion::new(8); + + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::call] + impl Pallet { + pub fn increment_value(_origin: OriginFor) -> DispatchResult { + Value::::mutate(|v| { + v.saturating_add(1) + }); + Ok(()) + } + } +} + +fn main() { +} diff --git a/substrate/frame/support/test/tests/split_ui/pass/storage/mod.rs b/substrate/frame/support/test/tests/split_ui/pass/storage/mod.rs new file mode 100644 index 0000000000000..26974a750dc3a --- /dev/null +++ b/substrate/frame/support/test/tests/split_ui/pass/storage/mod.rs @@ -0,0 +1,27 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use frame_support::pallet_macros::pallet_section; + +#[pallet_section] +mod storage { + #[pallet::storage] + pub type Value = StorageValue<_, u32, ValueQuery>; + + #[pallet::storage] + pub type Map = StorageMap<_, _, u32, u32, ValueQuery>; +}