From d1c115b6197bf6c45d5640594f0432e6c2781a4f Mon Sep 17 00:00:00 2001
From: Julian Eager <eagr@tutanota.com>
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 00000000000..3b3b5a4cb5f
--- /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 e5bfa2793cb..10e674c3cb1 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 00000000000..e8601587fac
--- /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<T>(_);
+
+ #[pallet::config]
+ pub trait Config: frame_system::Config {}
+
+ #[pallet::call]
+ impl<T: Config> Pallet<T> {
+ pub fn increment_value(_origin: OriginFor<T>) -> DispatchResult {
+ Value::<T>::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 00000000000..26974a750dc
--- /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<T> = StorageValue<_, u32, ValueQuery>;
+
+ #[pallet::storage]
+ pub type Map<T> = StorageMap<_, _, u32, u32, ValueQuery>;
+}
--
GitLab