ronantakizawa/codereview-bench huggingface dataset:
Repository: paritytech/substrate
File: frame/support/procedural/src/pallet/expand/doc_only.rs (Rust)

Code before:
					/// # Warning: Doc-Only
					///
					/// This function is an automatically generated, and is doc-only, uncallable
					/// stub. See the real version in
					#[ doc = #real ]
					pub fn #name<#type_impl_generics>(#args) { unreachable!(); }
				)
			})
			.collect::<proc_macro2::TokenStream>()
	} else {
		quote::quote!()
	};

	let storage_types = def
		.storages
		.iter()
		.map(|storage| {
			let storage_name = &storage.ident;
			let storage_type_docs = &storage.docs;
			let real = format!("[`pallet::{}`].", storage_name);
			quote::quote!(
				#( #[doc = #storage_type_docs] )*
				///
				/// # Warning: Doc-Only
				///
				/// This type is an automatically generated, and is doc-only. See the real version in
				#[ doc = #real ]
				pub struct #storage_name();
			)
		})
		.collect::<proc_macro2::TokenStream>();

	quote::quote!(
		/// Auto-generated docs-only module listing all defined storage types for this pallet,
		/// public or non-public.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes.
		///
		/// To see the actual storage type, find the a struct with the same name at the root of the
		/// pallet, in the list of [*Type Definitions*](../index.html#types).
		#[cfg(doc)]
		pub mod storage_types {
			use super::*;
			#storage_types
		}

		/// Auto-generated docs-only module listing all defined dispatchables for this pallet.
		///

Review comment (suggestion):
```suggestion
				/// This type is automatically generated, and is doc-only. See the real version in
```

Code after:
					/// # Warning: Doc-Only
					///
					/// This function is an automatically generated, and is doc-only, uncallable
					/// stub. See the real version in
					#[ doc = #real ]
					pub fn #name<#type_impl_generics>(#args) { unreachable!(); }
				)
			})
			.collect::<proc_macro2::TokenStream>()
	} else {
		quote::quote!()
	};

	let storage_types = def
		.storages
		.iter()
		.map(|storage| {
			let storage_name = &storage.ident;
			let storage_type_docs = &storage.docs;
			let real = format!("[`pallet::{}`].", storage_name);
			quote::quote!(
				#( #[doc = #storage_type_docs] )*
				///
				/// # Warning: Doc-Only
				///
				/// This type is automatically generated, and is doc-only. See the real version in
				#[ doc = #real ]
				pub struct #storage_name();
			)
		})
		.collect::<proc_macro2::TokenStream>();

	quote::quote!(
		/// Auto-generated docs-only module listing all (public and private) defined storage types
		/// for this pallet.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes.
		///
		/// To see the actual storage type, find a struct with the same name at the root of the
		/// pallet, in the list of [*Type Definitions*](../index.html#types).
		#[cfg(doc)]
		pub mod storage_types {
			use super::*;
			#storage_types
		}

		/// Auto-generated docs-only module listing all defined dispatchables for this pallet.
		///
Repository: paritytech/substrate
File: frame/support/procedural/src/pallet/expand/doc_only.rs (Rust)

Code before:
		.map(|storage| {
			let storage_name = &storage.ident;
			let storage_type_docs = &storage.docs;
			let real = format!("[`pallet::{}`].", storage_name);
			quote::quote!(
				#( #[doc = #storage_type_docs] )*
				///
				/// # Warning: Doc-Only
				///
				/// This type is automatically generated, and is doc-only. See the real version in
				#[ doc = #real ]
				pub struct #storage_name();
			)
		})
		.collect::<proc_macro2::TokenStream>();

	quote::quote!(
		/// Auto-generated docs-only module listing all (public and private) defined storage types
		/// for this pallet.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes.
		///
		/// To see the actual storage type, find the a struct with the same name at the root of the
		/// pallet, in the list of [*Type Definitions*](../index.html#types).
		#[cfg(doc)]
		pub mod storage_types {
			use super::*;
			#storage_types
		}

		/// Auto-generated docs-only module listing all defined dispatchables for this pallet.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes. To see the real version of each dispatchable, look for them in [`Pallet`] or
		/// [`Call`].
		#[cfg(doc)]
		pub mod dispatchables {
			use super::*;
			#dispatchables
		}
	)
}


Review comment (suggestion):
```suggestion
		/// To see the actual storage type, find a struct with the same name at the root of the
```

Code after:
		.map(|storage| {
			let storage_name = &storage.ident;
			let storage_type_docs = &storage.docs;
			let real = format!("[`pallet::{}`].", storage_name);
			quote::quote!(
				#( #[doc = #storage_type_docs] )*
				///
				/// # Warning: Doc-Only
				///
				/// This type is automatically generated, and is doc-only. See the real version in
				#[ doc = #real ]
				pub struct #storage_name();
			)
		})
		.collect::<proc_macro2::TokenStream>();

	quote::quote!(
		/// Auto-generated docs-only module listing all (public and private) defined storage types
		/// for this pallet.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes.
		///
		/// To see the actual storage type, find a struct with the same name at the root of the
		/// pallet, in the list of [*Type Definitions*](../index.html#types).
		#[cfg(doc)]
		pub mod storage_types {
			use super::*;
			#storage_types
		}

		/// Auto-generated docs-only module listing all defined dispatchables for this pallet.
		///
		/// # Warning: Doc-Only
		///
		/// Members of this module cannot be used directly and are only provided for documentation
		/// purposes. To see the real version of each dispatchable, look for them in [`Pallet`] or
		/// [`Call`].
		#[cfg(doc)]
		pub mod dispatchables {
			use super::*;
			#dispatchables
		}
	)
}
Repository: paritytech/substrate
File: frame/support/procedural/src/pallet/expand/config.rs (Rust)

Code before:
// 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 crate::pallet::Def;

///
/// * Generate default rust doc
pub fn expand_config(def: &mut Def) -> proc_macro2::TokenStream {
	let config = &def.config;
	let config_item = {
		let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[config.index];
		if let syn::Item::Trait(item) = item {
			item
		} else {
			unreachable!("Checked by config parser")
		}
	};

	config_item.attrs.insert(
		0,
		syn::parse_quote!(
			#[doc = r"Configuration trait of this pallet.

The main purpose of this trait is to act as an interface between this pallet and the runtime in
which it is embedded in. A type, function, or constant in this trait  is essentially left to be
configured by the runtime that includes this pallet.

Consequently, a runtime that wants to include this pallet mustimplement this trait."
			]
		),
	);

	Default::default()
}


Review comment (suggestion):
```suggestion
which it is embedded in. A type, function, or constant in this trait is essentially left to be
```
(:

Code after:
// 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 crate::pallet::Def;

///
/// * Generate default rust doc
pub fn expand_config(def: &mut Def) -> proc_macro2::TokenStream {
	let config = &def.config;
	let config_item = {
		let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[config.index];
		if let syn::Item::Trait(item) = item {
			item
		} else {
			unreachable!("Checked by config parser")
		}
	};

	config_item.attrs.insert(
		0,
		syn::parse_quote!(
			#[doc = r"Configuration trait of this pallet.

The main purpose of this trait is to act as an interface between this pallet and the runtime in
which it is embedded in. A type, function, or constant in this trait is essentially left to be
configured by the runtime that includes this pallet.

Consequently, a runtime that wants to include this pallet must implement this trait."
			]
		),
	);

	Default::default()
}
Repository: paritytech/substrate
File: frame/support/procedural/src/pallet/expand/config.rs (Rust)

Code before:

use crate::pallet::Def;

///
/// * Generate default rust doc
pub fn expand_config(def: &mut Def) -> proc_macro2::TokenStream {
	let config = &def.config;
	let config_item = {
		let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[config.index];
		if let syn::Item::Trait(item) = item {
			item
		} else {
			unreachable!("Checked by config parser")
		}
	};

	config_item.attrs.insert(
		0,
		syn::parse_quote!(
			#[doc = r"Configuration trait of this pallet.

The main purpose of this trait is to act as an interface between this pallet and the runtime in
which it is embedded in. A type, function, or constant in this trait  is essentially left to be
configured by the runtime that includes this pallet.

Consequently, a runtime that wants to include this pallet mustimplement this trait."
			]
		),
	);

	Default::default()
}


Review comment (suggestion):
```suggestion
Consequently, a runtime that wants to include this pallet must implement this trait."
```

Code after:

use crate::pallet::Def;

///
/// * Generate default rust doc
pub fn expand_config(def: &mut Def) -> proc_macro2::TokenStream {
	let config = &def.config;
	let config_item = {
		let item = &mut def.item.content.as_mut().expect("Checked by def parser").1[config.index];
		if let syn::Item::Trait(item) = item {
			item
		} else {
			unreachable!("Checked by config parser")
		}
	};

	config_item.attrs.insert(
		0,
		syn::parse_quote!(
			#[doc = r"Configuration trait of this pallet.

The main purpose of this trait is to act as an interface between this pallet and the runtime in
which it is embedded in. A type, function, or constant in this trait is essentially left to be
configured by the runtime that includes this pallet.

Consequently, a runtime that wants to include this pallet must implement this trait."
			]
		),
	);

	Default::default()
}
Repository: paritytech/substrate
File: primitives/runtime/src/traits.rs (Rust)

Code before:

	/// The payload we carry for signed extrinsics.
	///
	/// Usually it will contain a `Signature` and
	/// may include some additional data that are specific to signed
	/// extrinsics.
	type SignaturePayload: SignaturePayload;

	/// Is this `Extrinsic` signed?
	/// If no information are available about signed/unsigned, `None` should be returned.
	fn is_signed(&self) -> Option<bool> {
		None
	}

	/// Create new instance of the extrinsic.
	///
	/// Extrinsics can be split into:
	/// 1. Inherents (no signature; created by validators during block production)
	/// 2. Unsigned Transactions (no signature; represent "system calls" or other special kinds of
	/// calls) 3. Signed Transactions (with signature; a regular transactions with known origin)
	fn new(_call: Self::Call, _signed_data: Option<Self::SignaturePayload>) -> Option<Self> {
		None
	}
}

/// Something that acts like a `SignaturePayload` of an `Extrinsic.
pub trait SignaturePayload {
	/// The type of the address that signed the extrinsic.
	///
	/// Particular to a signed extrinsic.
	type SignatureAddress: TypeInfo;

	/// The signature type of the extrinsic.
	///
	/// Particular to a signed extrinsic.
	type Signature: TypeInfo;

	/// The additional data that is specific to the signed extrinsic.
	///
	/// Particular to a signed extrinsic.
	type SignatureExtra: TypeInfo;
}

impl SignaturePayload for () {
	type SignatureAddress = ();
	type Signature = ();
	type SignatureExtra = ();
}

/// Implementor is an [`Extrinsic`] and provides metadata about this extrinsic.
pub trait ExtrinsicMetadata {

Review comment (suggestion):
```suggestion
/// Something that acts like a [`SignaturePayload`](Extrinsic::SignaturePayload) of an [`Extrinsic`].
```

Code after:
		<Self as Hasher>::hash(s)
	}

	/// Produce the hash of some codec-encodable value.
	fn hash_of<S: Encode>(s: &S) -> Self::Output {
		Encode::using_encoded(s, <Self as Hasher>::hash)
	}

	/// The ordered Patricia tree root of the given `input`.
	fn ordered_trie_root(input: Vec<Vec<u8>>, state_version: StateVersion) -> Self::Output;

	/// The Patricia tree root of the given mapping.
	fn trie_root(input: Vec<(Vec<u8>, Vec<u8>)>, state_version: StateVersion) -> Self::Output;
}

/// Super trait with all the attributes for a hashing output.
pub trait HashOutput:
	Member
	+ MaybeSerializeDeserialize
	+ MaybeDisplay
	+ MaybeFromStr
	+ Debug
	+ sp_std::hash::Hash
	+ AsRef<[u8]>
	+ AsMut<[u8]>
	+ Copy
	+ Ord
	+ Default
	+ Encode
	+ Decode
	+ EncodeLike
	+ MaxEncodedLen
	+ TypeInfo
{
}

impl<T> HashOutput for T where
	T: Member
		+ MaybeSerializeDeserialize
		+ MaybeDisplay
		+ MaybeFromStr
		+ Debug
		+ sp_std::hash::Hash
		+ AsRef<[u8]>
		+ AsMut<[u8]>
		+ Copy
		+ Ord
		+ Default
		+ Encode
		+ Decode
		+ EncodeLike
Repository: paritytech/substrate
File: frame/glutton/src/lib.rs (Rust)

Code before:
	#[pallet::storage]
	pub(crate) type TrashDataCount<T: Config> = StorageValue<_, u32, ValueQuery>;

	#[pallet::genesis_config]
	pub struct GenesisConfig {
		pub compute: Perbill,
		pub storage: Perbill,
		pub trash_data_count: u32,
	}

	impl Default for GenesisConfig {
		fn default() -> Self {
			Self {
				compute: Default::default(),
				storage: Default::default(),
				trash_data_count: Default::default(),
			}
		}
	}

	#[pallet::genesis_build]
	impl<T: Config> GenesisBuild<T> for GenesisConfig {
		fn build(&self) {
			assert!(
				self.trash_data_count <= MAX_TRASH_DATA_ENTRIES,
				"number of TrashData entries can not be bigger than {:?}",
				MAX_TRASH_DATA_ENTRIES
			);

			(0..self.trash_data_count)
				.for_each(|i| TrashData::<T>::insert(i, Pallet::<T>::gen_value(i)));

			TrashDataCount::<T>::set(self.trash_data_count);

			<Compute<T>>::put(self.compute);
			<Storage<T>>::put(self.storage);
		}
	}

	#[pallet::hooks]
	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
		fn integrity_test() {
			assert!(
				!T::WeightInfo::waste_ref_time_iter(1).ref_time().is_zero(),
				"Weight zero; would get stuck in an infinite loop"
			);
			assert!(
				!T::WeightInfo::waste_proof_size_some(1).proof_size().is_zero(),
				"Weight zero; would get stuck in an infinite loop"
			);
		}

Review comment (suggestion):
```suggestion
				"number of TrashData entries cannot be bigger than {:?}",
```

Code after:
	#[pallet::storage]
	pub(crate) type TrashDataCount<T: Config> = StorageValue<_, u32, ValueQuery>;

	#[pallet::genesis_config]
	pub struct GenesisConfig {
		pub compute: Perbill,
		pub storage: Perbill,
		pub trash_data_count: u32,
	}

	impl Default for GenesisConfig {
		fn default() -> Self {
			Self {
				compute: Default::default(),
				storage: Default::default(),
				trash_data_count: Default::default(),
			}
		}
	}

	#[pallet::genesis_build]
	impl<T: Config> GenesisBuild<T> for GenesisConfig {
		fn build(&self) {
			assert!(
				self.trash_data_count <= MAX_TRASH_DATA_ENTRIES,
				"number of TrashData entries cannot be bigger than {:?}",
				MAX_TRASH_DATA_ENTRIES
			);

			(0..self.trash_data_count)
				.for_each(|i| TrashData::<T>::insert(i, Pallet::<T>::gen_value(i)));

			TrashDataCount::<T>::set(self.trash_data_count);

			<Compute<T>>::put(self.compute);
			<Storage<T>>::put(self.storage);
		}
	}

	#[pallet::hooks]
	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
		fn integrity_test() {
			assert!(
				!T::WeightInfo::waste_ref_time_iter(1).ref_time().is_zero(),
				"Weight zero; would get stuck in an infinite loop"
			);
			assert!(
				!T::WeightInfo::waste_proof_size_some(1).proof_size().is_zero(),
				"Weight zero; would get stuck in an infinite loop"
			);
		}
Repository: paritytech/substrate
File: primitives/runtime-interface/proc-macro/src/runtime_interface/host_function_interface.rs (Rust)

Code before:
		},
		ReturnType::Default => quote! {
			let __result__ = Ok(__result__);
		},
	};

	let convert_return_value_static_ffi_to_dynamic_ffi = match &method.sig.output {
		ReturnType::Type(_, _) => quote! {
			let __result__ = Ok(Some(#crate_::sp_wasm_interface::IntoValue::into_value(__result__)));
		},
		ReturnType::Default => quote! {
			let __result__ = Ok(None);
		},
	};

	if is_wasm_only {
		host_names_with_ref.push(quote! {
			__function_context__
		});
	}

	let cfg_attrs: Vec<_> = method
		.attrs
		.iter()
		.filter(|a| a.path().is_ident("cfg"))
		.map(|a| a.clone())
		.collect();

	let implementation = quote! {
		#(#cfg_attrs)*
		#[cfg(feature = "std")]
		struct #struct_name;

		#(#cfg_attrs)*
		#[cfg(feature = "std")]
		impl #struct_name {
			fn call(
				__function_context__: &mut dyn #crate_::sp_wasm_interface::FunctionContext,
				#(#ffi_args_prototype),*
			) -> std::result::Result<#ffi_return_ty, String> {
				#(#convert_args_static_ffi_to_host)*
				let __result__ = #fn_name(#(#host_names_with_ref),*);
				#(#copy_data_into_ref_mut_args)*
				#convert_return_value_host_to_static_ffi
				__result__
			}
		}

		#(#cfg_attrs)*
		#[cfg(feature = "std")]
		impl #crate_::sp_wasm_interface::Function for #struct_name {

Review comment (suggestion):
```suggestion
		.cloned()
```

(I *think* this should compile, but please check to be sure.)

Code after:
		},
		ReturnType::Default => quote! {
			let __result__ = Ok(__result__);
		},
	};

	let convert_return_value_static_ffi_to_dynamic_ffi = match &method.sig.output {
		ReturnType::Type(_, _) => quote! {
			let __result__ = Ok(Some(#crate_::sp_wasm_interface::IntoValue::into_value(__result__)));
		},
		ReturnType::Default => quote! {
			let __result__ = Ok(None);
		},
	};

	if is_wasm_only {
		host_names_with_ref.push(quote! {
			__function_context__
		});
	}

	let cfg_attrs: Vec<_> =
		method.attrs.iter().filter(|a| a.path().is_ident("cfg")).cloned().collect();
	if version > 1 && !cfg_attrs.is_empty() {
		return Err(Error::new(
			method.span(),
			"Conditional compilation is not supported for versioned functions",
		))
	}

	let implementation = quote! {
		#(#cfg_attrs)*
		#[cfg(feature = "std")]
		struct #struct_name;

		#(#cfg_attrs)*
		#[cfg(feature = "std")]
		impl #struct_name {
			fn call(
				__function_context__: &mut dyn #crate_::sp_wasm_interface::FunctionContext,
				#(#ffi_args_prototype),*
			) -> std::result::Result<#ffi_return_ty, String> {
				#(#convert_args_static_ffi_to_host)*
				let __result__ = #fn_name(#(#host_names_with_ref),*);
				#(#copy_data_into_ref_mut_args)*
				#convert_return_value_host_to_static_ffi
				__result__
			}
		}

		#(#cfg_attrs)*
Repository: paritytech/substrate
File: frame/support/procedural/src/construct_runtime/expand/config.rs (Rust)

Code before:
				#path::__substrate_genesis_config_check::is_genesis_config_defined!(#pallet_name);
				#[cfg(feature = "std")]
				#path::__substrate_genesis_config_check::is_std_enabled_for_genesis!(#pallet_name, #path_str);
			});
		}
	}

	quote! {
		#( #query_genesis_config_part_macros )*

		#types

		#[cfg(any(feature = "std", test))]
		use #scrate::serde as __genesis_config_serde_import__;
		#[cfg(any(feature = "std", test))]
		#[derive(#scrate::serde::Serialize, #scrate::serde::Deserialize, Default)]
		#[serde(rename_all = "camelCase")]
		#[serde(deny_unknown_fields)]
		#[serde(crate = "__genesis_config_serde_import__")]
		pub struct RuntimeGenesisConfig {
			#fields
		}

		// TODO [#14065]
		// - deprecate it
		// - later, remove it
		#[cfg(any(feature = "std", test))]
		pub type GenesisConfig = RuntimeGenesisConfig;

		#[cfg(any(feature = "std", test))]
		impl #scrate::sp_runtime::BuildStorage for RuntimeGenesisConfig {
			fn assimilate_storage(
				&self,
				storage: &mut #scrate::sp_runtime::Storage,
			) -> std::result::Result<(), String> {
				#build_storage_calls

				#scrate::BasicExternalities::execute_with_storage(storage, || {
					<AllPalletsWithSystem as #scrate::traits::OnGenesis>::on_genesis();
				});

				Ok(())
			}
		}
	}
}

fn expand_config_types(
	attr: &TokenStream,
	runtime: &Ident,
	decl: &Pallet,

Review comment (suggestion):
Personally I think it would be cleaner to not put TODO or deprecation steps in the code comment itself, and just list them in the issue.

The code comment could say something like "GenesisConfig is in the process of being deprecated [issue link]"

Code after:
				#path::__substrate_genesis_config_check::is_genesis_config_defined!(#pallet_name);
				#[cfg(feature = "std")]
				#path::__substrate_genesis_config_check::is_std_enabled_for_genesis!(#pallet_name, #path_str);
			});
		}
	}

	quote! {
		#( #query_genesis_config_part_macros )*

		#types

		#[cfg(any(feature = "std", test))]
		use #scrate::serde as __genesis_config_serde_import__;
		#[cfg(any(feature = "std", test))]
		#[derive(#scrate::serde::Serialize, #scrate::serde::Deserialize, Default)]
		#[serde(rename_all = "camelCase")]
		#[serde(deny_unknown_fields)]
		#[serde(crate = "__genesis_config_serde_import__")]
		pub struct RuntimeGenesisConfig {
			#fields
		}

		// GenesisConfig is in the process of being deprecated [https://github.com/paritytech/substrate/issues/14065]
		#[cfg(any(feature = "std", test))]
		pub type GenesisConfig = RuntimeGenesisConfig;

		#[cfg(any(feature = "std", test))]
		impl #scrate::sp_runtime::BuildStorage for RuntimeGenesisConfig {
			fn assimilate_storage(
				&self,
				storage: &mut #scrate::sp_runtime::Storage,
			) -> std::result::Result<(), String> {
				#build_storage_calls

				#scrate::BasicExternalities::execute_with_storage(storage, || {
					<AllPalletsWithSystem as #scrate::traits::OnGenesis>::on_genesis();
				});

				Ok(())
			}
		}
	}
}

fn expand_config_types(
	attr: &TokenStream,
	runtime: &Ident,
	decl: &Pallet,
	config: &Ident,
	part_is_generic: bool,