Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
37c0a57
Parameterize CompactForm String for optional SCALE impl
ascjones Dec 7, 2020
643f09d
Merge remote-tracking branch 'origin/master' into aj-compact-string
dvdplm Dec 14, 2020
9a7ccbf
Fix no-std compilation
dvdplm Dec 14, 2020
254fee1
Obey the fmt
dvdplm Dec 14, 2020
e74e4f9
Introduce String trait for Form
ascjones Dec 16, 2020
7860c79
Rename "Compact" to "Frozen" (and associated fallout)
dvdplm Dec 16, 2020
579f958
Docs cleanup and more renames
dvdplm Dec 17, 2020
8333e5a
Cleanup
dvdplm Dec 17, 2020
2818f7b
More cleanup
dvdplm Dec 17, 2020
7706a38
Merge branch 'aj-compact-string' into dp-rename-compact-to-frozen
dvdplm Dec 17, 2020
e03a2cd
obey the fmt
dvdplm Dec 17, 2020
3a95663
Add a `compact` flag to `Field` to indicate that this type is to be e…
dvdplm Dec 17, 2020
004e107
Clippy warnings
dvdplm Dec 17, 2020
93a9aeb
Acommodate older clippy
dvdplm Dec 17, 2020
6569e50
Derive (scale) compact fields
dvdplm Dec 28, 2020
f098101
Merge branch 'master' into dp-flag-types-as-compact
dvdplm Jan 4, 2021
b43cdfc
Use utils from parity-scale-codec-derive
dvdplm Jan 4, 2021
eda2769
Merge remote-tracking branch 'origin/master' into dp-flag-types-as-co…
dvdplm Jan 5, 2021
6321da9
Merge branch 'dp-flag-types-as-compact' into dp-handle-scale-skip
dvdplm Jan 5, 2021
1d32a38
Merge remote-tracking branch 'origin/master' into dp-handle-scale-skip
dvdplm Jan 19, 2021
d1700b2
fmt
dvdplm Jan 19, 2021
3387355
Merge branch 'master' into dp-handle-scale-skip
dvdplm Feb 17, 2021
d29c9be
Attempt to fix CI
dvdplm Feb 18, 2021
0426479
FIx CI take 2
dvdplm Feb 18, 2021
0a97eb1
Merge branch 'master' into dp-handle-scale-skip
dvdplm Mar 2, 2021
81e3519
Merge branch 'master' into dp-handle-scale-skip
dvdplm Mar 17, 2021
e8d2a74
Use is_compact from utils
dvdplm Mar 17, 2021
d9239c5
Fn is enough
dvdplm Mar 17, 2021
98292c5
Doc tweaks
dvdplm Mar 18, 2021
71d7e11
Add tests for enums
dvdplm Mar 18, 2021
00aee2f
Add test for indexed enum
dvdplm Mar 22, 2021
a31acc5
Oops
dvdplm Mar 22, 2021
e70425f
Update derive/src/utils.rs
dvdplm Mar 23, 2021
abdb081
Review feedback
dvdplm Mar 23, 2021
c53eb38
fmt
dvdplm Mar 23, 2021
1c1802e
Better error message, clearer bad syntax trubuild-test
dvdplm Mar 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions derive/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub fn variant_index(v: &Variant, i: usize) -> TokenStream {
if let Lit::Int(ref v) = nv.lit {
let byte = v
.base10_parse::<u8>()
.expect("Internal error, index attribute must have been checked");
.expect("Internal error. `#[codec(index = …)]` attribute syntax must be checked in `parity-scale-codec`. This is a bug.");
return Some(byte)
}
}
Expand All @@ -56,7 +56,7 @@ pub fn variant_index(v: &Variant, i: usize) -> TokenStream {
})
}

// /// Look for a `#[codec(compact)]` outer attribute on the given `Field`.
/// Look for a `#[codec(compact)]` outer attribute on the given `Field`.
pub fn is_compact(field: &syn::Field) -> bool {
let outer_attrs = field
.attrs
Expand Down Expand Up @@ -97,7 +97,7 @@ where
if attr.path.is_ident("codec") {
if let Meta::List(ref meta_list) = attr
.parse_meta()
.expect("Internal error, parse_meta must have been checked")
.expect("Internal error. `#[codec(…)]` attributes must be checked in `parity-scale-codec`. This is a bug.")
{
return meta_list.nested.iter().filter_map(pred.clone()).next()
}
Expand Down
100 changes: 0 additions & 100 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,103 +183,3 @@ fn basic_struct_with_phantoms() {

assert_type!(SomeStruct<bool>, struct_bool_type_info);
}

#[test]
fn basic_c_like_enum() {
#[allow(unused)]
enum BasicCStyleEnum {
A,
B,
}

impl TypeInfo for BasicCStyleEnum {
type Identity = Self;

fn type_info() -> Type {
Type::builder()
.path(Path::new("BasicCStyleEnum", module_path!()))
.variant(Variants::fieldless().variant("A", 0).variant("B", 1))
}
}

let ty = Type::builder()
.path(
Path::from_segments(vec!["scale_info", "tests", "BasicCStyleEnum"]).unwrap(),
)
.variant(Variants::fieldless().variant("A", 0).variant("B", 1));

assert_type!(BasicCStyleEnum, ty);
}

#[test]
fn c_like_enum_with_discriminant_override() {
#[allow(unused)]
enum CStyleEnum {
A = 3,
B = 0,
}

impl TypeInfo for CStyleEnum {
type Identity = Self;

fn type_info() -> Type {
Type::builder()
.path(Path::new("CStyleEnum", module_path!()))
.variant(Variants::fieldless().variant("A", 3).variant("B", 0))
}
}

let ty = Type::builder()
.path(Path::from_segments(vec!["scale_info", "tests", "CStyleEnum"]).unwrap())
.variant(Variants::fieldless().variant("A", 3).variant("B", 0));

assert_type!(CStyleEnum, ty);
}

#[test]
fn basic_enum() {
#[allow(unused)]
enum RustEnum {
A(bool),
B { b: u8 },
C(u16, u32),
D,
}
impl TypeInfo for RustEnum {
type Identity = Self;

fn type_info() -> Type {
Type::builder()
.path(Path::new("RustEnum", module_path!()))
.variant(
Variants::with_fields()
.variant("A", Fields::unnamed().field_of::<bool>("bool"))
.variant("B", Fields::named().field_of::<u8>("b", "bool"))
.variant(
"C",
Fields::unnamed()
.field_of::<u16>("u16")
.field_of::<u32>("u32"),
)
.variant_unit("D"),
)
}
}

let ty = Type::builder()
.path(Path::new("RustEnum", module_path!()))
.variant(
Variants::with_fields()
.variant("A", Fields::unnamed().field_of::<bool>("bool"))
.variant("B", Fields::named().field_of::<u8>("b", "bool"))
.variant(
"C",
Fields::unnamed()
.field_of::<u16>("u16")
.field_of::<u32>("u32"),
)
.variant_unit("D"),
);

assert_type!(RustEnum, ty);
}
2 changes: 1 addition & 1 deletion src/ty/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use serde::{
/// Monday,
/// Tuesday,
/// Wednesday,
/// Thursday = 42, // Also allows to manually set the discriminant!
/// Thursday = 42, // Allows setting the discriminant explicitly
/// Friday,
/// Saturday,
/// Sunday,
Expand Down
11 changes: 8 additions & 3 deletions test_suite/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,18 @@ fn c_like_enum_derive_with_scale_index_set() {
B = 10,
#[codec(index = 13)]
C,
D,
#[codec(index = 14)]
E = 15,
}

let ty = Type::builder().path(Path::new("E", "derive")).variant(
Variants::fieldless()
.variant("A", 0u64)
.variant("B", 10u64)
.variant("C", 13u64),
.variant("A", 0)
.variant("B", 10)
.variant("C", 13)
.variant("D", 3)
.variant("E", 14),
);

assert_type!(E, ty);
Expand Down
2 changes: 1 addition & 1 deletion test_suite/tests/ui/fail_with_invalid_codec_attrs.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ error: proc-macro derive panicked
11 | #[derive(TypeInfo, Encode)]
| ^^^^^^^^
|
= help: message: Internal error, parse_meta must have been checked: Error("expected literal")
= help: message: Internal error. `#[codec(…)]` attributes must be checked in `parity-scale-codec`. This is a bug.: Error("expected literal")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't 100% get this - the errors above were generated by parity-scale-codec so the check appears to have happened correctly - so this error is not necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it’s there to chastise sloppy developers and should never reach users. It means that the attribute syntax checks never happened or are buggy; the derive author has to explicitly make a method call so there’s nothing stopping human error.
We could unwrap here but that’s not better.
The trybuild is here as a guard against buggy parity-scale-codec releases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should never reach users

Forgive me 🙈 , but doesn't this trybuild demonstrate that this error does in fact reach the user even though the syntax checks did happen?

Copy link
Copy Markdown
Contributor Author

@dvdplm dvdplm Mar 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right and I am wrong. First of all I'm heaping on a bunch of different syntax errors in the same enum which makes it hard to understand what is going on. If I use a single-syntax error at a time it's easier to follow.

#[derive(Encode, TypeInfo)]
enum EnumAttrsValidation {
    #[codec(index = a)]
    Thing(u32),
}

Gives this error:

error: proc-macro derive panicked
  --> $DIR/fail_with_invalid_codec_attrs.rs:17:18
   |
17 | #[derive(Encode, TypeInfo)]
   |                  ^^^^^^^^
   |
   = help: message: Internal error. `#[codec(…)]` attributes must be checked in `parity-scale-codec`. This is a bug.: Error("expected literal")

error: expected literal
  --> $DIR/fail_with_invalid_codec_attrs.rs:19:21
   |
19 |     #[codec(index = a)]
   |                     ^

So parity-scale-codec is giving the proper error message back, whereas scale-info just panics. What I was trying to say with this test is that scale-info will not try to check attributes from parity-scale-codec.

We have no way of knowing if the Encode macro ran or how it went, so we can either panic or re-implement their attribute checking and print the proper error twice.

I think we should panic but my error message must change. How about: scale-info: Bad index in `#[codec(index = …)]`: Error("expected literal").

Wdyt?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, maybe could mention somehow to see the other error generated by parity-scale-codec.