Skip to content

Commit

Permalink
make types within generate_solution_type macro explicit (paritytech…
Browse files Browse the repository at this point in the history
…#8447)

* make types within `generate_solution_type` macro explicit

Closes paritytech#8444.

Just changes the parsing logic for that macro; does not change any
emitted code. The associated types associated with the macro now
require explicit, keyword-style declaration.

**Old**:

```rust
sp_npos_elections::generate_solution_type!(
	#[compact]
	pub struct TestCompact::<VoterIndex, TargetIndex, PerU16>(16)
);
```

**New**:

```rust
sp_npos_elections::generate_solution_type!(
	#[compact]
	pub struct TestCompact::<VoterIndex = VoterIndex, CandidateIndex = TargetIndex, Accuracy = PerU16>(16)
);
```

* un-ignore doc-tests

* use new form in bin/node/runtime/

* rename CandidateIndex -> TargetIndex

* add tests demonstrating some potential compile failures
  • Loading branch information
coriolinus authored and hirschenberger committed Apr 14, 2021
1 parent de9c90c commit 0de37ac
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 16 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,8 +519,11 @@ parameter_types! {

sp_npos_elections::generate_solution_type!(
#[compact]
pub struct NposCompactSolution16::<u32, u16, sp_runtime::PerU16>(16)
// -------------------- ^^ <NominatorIndex, ValidatorIndex, Accuracy>
pub struct NposCompactSolution16::<
VoterIndex = u32,
TargetIndex = u16,
Accuracy = sp_runtime::PerU16,
>(16)
);

impl pallet_election_provider_multi_phase::Config for Runtime {
Expand Down
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(crate) type TargetIndex = u16;

sp_npos_elections::generate_solution_type!(
#[compact]
pub struct TestCompact::<VoterIndex, TargetIndex, PerU16>(16)
pub struct TestCompact::<VoterIndex = VoterIndex, TargetIndex = TargetIndex, Accuracy = PerU16>(16)
);

/// All events of this pallet.
Expand Down
6 changes: 6 additions & 0 deletions primitives/npos-elections/compact/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ syn = { version = "1.0.58", features = ["full", "visit"] }
quote = "1.0"
proc-macro2 = "1.0.6"
proc-macro-crate = "1.0.0"

[dev-dependencies]
parity-scale-codec = "2.0.1"
sp-arithmetic = { path = "../../arithmetic" }
sp-npos-elections = { path = ".." }
trybuild = "1.0.41"
46 changes: 38 additions & 8 deletions primitives/npos-elections/compact/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,14 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error {
/// For example, the following generates a public struct with name `TestSolution` with `u16` voter
/// type, `u8` target type and `Perbill` accuracy with maximum of 8 edges per voter.
///
/// ```ignore
/// generate_solution_type!(pub struct TestSolution<u16, u8, Perbill>::(8))
/// ```
/// # use sp_npos_elections_compact::generate_solution_type;
/// # use sp_arithmetic::per_things::Perbill;
/// generate_solution_type!(pub struct TestSolution::<
/// VoterIndex = u16,
/// TargetIndex = u8,
/// Accuracy = Perbill,
/// >(8));
/// ```
///
/// The given struct provides function to convert from/to Assignment:
Expand All @@ -65,11 +71,13 @@ pub(crate) fn syn_err(message: &'static str) -> syn::Error {
/// lead to many 0s in the solution. If prefixed with `#[compact]`, then a custom compact encoding
/// for numbers will be used, similar to how `parity-scale-codec`'s `Compact` works.
///
/// ```ignore
/// ```
/// # use sp_npos_elections_compact::generate_solution_type;
/// # use sp_arithmetic::per_things::Perbill;
/// generate_solution_type!(
/// #[compact]
/// pub struct TestSolutionCompact<u16, u8, Perbill>::(8)
/// )
/// pub struct TestSolutionCompact::<VoterIndex = u16, TargetIndex = u8, Accuracy = Perbill>(8)
/// );
/// ```
#[proc_macro]
pub fn generate_solution_type(item: TokenStream) -> TokenStream {
Expand Down Expand Up @@ -386,7 +394,7 @@ fn check_compact_attr(input: ParseStream) -> Result<bool> {
}
}

/// #[compact] pub struct CompactName::<u32, u32, u32>()
/// #[compact] pub struct CompactName::<VoterIndex = u32, TargetIndex = u32, Accuracy = u32>()
impl Parse for SolutionDef {
fn parse(input: ParseStream) -> syn::Result<Self> {
// optional #[compact]
Expand All @@ -405,9 +413,22 @@ impl Parse for SolutionDef {
return Err(syn_err("Must provide 3 generic args."))
}

let mut types: Vec<syn::Type> = generics.args.iter().map(|t|
let expected_types = ["VoterIndex", "TargetIndex", "Accuracy"];

let mut types: Vec<syn::Type> = generics.args.iter().zip(expected_types.iter()).map(|(t, expected)|
match t {
syn::GenericArgument::Type(ty) => Ok(ty.clone()),
syn::GenericArgument::Type(ty) => {
// this is now an error
Err(syn::Error::new_spanned(ty, format!("Expected binding: `{} = ...`", expected)))
},
syn::GenericArgument::Binding(syn::Binding{ident, ty, ..}) => {
// check that we have the right keyword for this position in the argument list
if ident == expected {
Ok(ty.clone())
} else {
Err(syn::Error::new_spanned(ident, format!("Expected `{}`", expected)))
}
}
_ => Err(syn_err("Wrong type of generic provided. Must be a `type`.")),
}
).collect::<Result<_>>()?;
Expand Down Expand Up @@ -436,3 +457,12 @@ impl Parse for SolutionDef {
fn field_name_for(n: usize) -> Ident {
Ident::new(&format!("{}{}", PREFIX, n), Span::call_site())
}

#[cfg(test)]
mod tests {
#[test]
fn ui_fail() {
let cases = trybuild::TestCases::new();
cases.compile_fail("tests/ui/fail/*.rs");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use sp_npos_elections_compact::generate_solution_type;

generate_solution_type!(pub struct TestSolution::<
VoterIndex = u16,
TargetIndex = u8,
Perbill,
>(8));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Expected binding: `Accuracy = ...`
--> $DIR/missing_accuracy.rs:6:2
|
6 | Perbill,
| ^^^^^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use sp_npos_elections_compact::generate_solution_type;

generate_solution_type!(pub struct TestSolution::<
VoterIndex = u16,
u8,
Accuracy = Perbill,
>(8));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Expected binding: `TargetIndex = ...`
--> $DIR/missing_target.rs:5:2
|
5 | u8,
| ^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use sp_npos_elections_compact::generate_solution_type;

generate_solution_type!(pub struct TestSolution::<
u16,
TargetIndex = u8,
Accuracy = Perbill,
>(8));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Expected binding: `VoterIndex = ...`
--> $DIR/missing_voter.rs:4:2
|
4 | u16,
| ^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use sp_npos_elections_compact::generate_solution_type;

generate_solution_type!(pub struct TestSolution::<
u16,
u8,
Perbill,
>(8));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Expected binding: `VoterIndex = ...`
--> $DIR/no_annotations.rs:4:2
|
4 | u16,
| ^^^
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use sp_npos_elections_compact::generate_solution_type;

generate_solution_type!(pub struct TestSolution::<
TargetIndex = u16,
VoterIndex = u8,
Accuracy = Perbill,
>(8));

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Expected `VoterIndex`
--> $DIR/swap_voter_target.rs:4:2
|
4 | TargetIndex = u16,
| ^^^^^^^^^^^
6 changes: 5 additions & 1 deletion primitives/npos-elections/fuzzer/src/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ use sp_npos_elections::sp_arithmetic::Percent;
use sp_runtime::codec::{Encode, Error};

fn main() {
generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::<u32, u32, Percent>(16));
generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::<
VoterIndex = u32,
TargetIndex = u32,
Accuracy = Percent,
>(16));
loop {
fuzz!(|fuzzer_data: &[u8]| {
let result_decoded: Result<InnerTestSolutionCompact, Error> =
Expand Down
20 changes: 16 additions & 4 deletions primitives/npos-elections/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,11 @@ mod solution_type {

type TestAccuracy = Percent;

generate_solution_type!(pub struct TestSolutionCompact::<u32, u8, TestAccuracy>(16));
generate_solution_type!(pub struct TestSolutionCompact::<
VoterIndex = u32,
TargetIndex = u8,
Accuracy = TestAccuracy,
>(16));

#[allow(dead_code)]
mod __private {
Expand All @@ -1158,15 +1162,19 @@ mod solution_type {
use sp_arithmetic::Percent;
generate_solution_type!(
#[compact]
struct InnerTestSolutionCompact::<u32, u8, Percent>(12)
struct InnerTestSolutionCompact::<VoterIndex = u32, TargetIndex = u8, Accuracy = Percent>(12)
);
}

#[test]
fn solution_struct_works_with_and_without_compact() {
// we use u32 size to make sure compact is smaller.
let without_compact = {
generate_solution_type!(pub struct InnerTestSolution::<u32, u32, Percent>(16));
generate_solution_type!(pub struct InnerTestSolution::<
VoterIndex = u32,
TargetIndex = u32,
Accuracy = Percent,
>(16));
let compact = InnerTestSolution {
votes1: vec![(2, 20), (4, 40)],
votes2: vec![
Expand All @@ -1180,7 +1188,11 @@ mod solution_type {
};

let with_compact = {
generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::<u32, u32, Percent>(16));
generate_solution_type!(#[compact] pub struct InnerTestSolutionCompact::<
VoterIndex = u32,
TargetIndex = u32,
Accuracy = Percent,
>(16));
let compact = InnerTestSolutionCompact {
votes1: vec![(2, 20), (4, 40)],
votes2: vec![
Expand Down

0 comments on commit 0de37ac

Please sign in to comment.