Fix derive(MaxEncodedLen) with codec(compact/encoded_as) by zdave-parity · Pull Request #754 · paritytech/parity-scale-codec

zdave-parity · 2025-07-28T23:12:07Z

Fixes #302.

zdave-parity · 2025-07-28T23:13:36Z

derive/src/utils.rs

 				if let Expr::Lit(ExprLit { lit: Lit::Str(ref s), .. }) = nv.value {
 					return Some(
-						TokenStream::from_str(&s.value())
+						s.parse()


AFAICT the span of s is properly preserved when using s.parse(). This improves error messages which refer to the encoded-as type.

zdave-parity · 2025-07-28T23:19:39Z

derive/src/max_encoded_len.rs

-	//
-	// We match the span of each field to the span of the corresponding
-	// `max_encoded_len` call. This way, if one field's type doesn't implement
-	// `MaxEncodedLen`, the compiler's error message will underline which field


This was just wrong AFAICT. quote_spanned applies the given span only to tokens created by quote_spanned itself. The error message the compiler produces when MaxEncodedLen is not implemented refers to the non-implementing type. The type tokens were not created by quote_spanned and so the span provided had no bearing on the error message.

The old code was using the span of each field, aka what is written above. You say this did not worked?

The quoting method has no effect on what is underlined in the MaxEncodedLen-not-implemented error message. The correct span is already attached to field.ty, and both quote and quote_spanned will preserve this. I wouldn't say the old code was broken as such, just misleading. The quote_spanned was pointless and the comment was inaccurate. FWIW the change in utils.rs is required to get the correct span attached to the tokens returned by get_encoded_as_type. Without that change, the MaxEncodedLen-not-implemented error message just underlines the MaxEncodedLen derive for encoded_as fields. Note that this is the case whether or not quote_spanned is used.

bkchr

Looks good, but please add some test

bkchr · 2025-07-29T09:31:02Z

derive/src/max_encoded_len.rs

-	//
-	// We match the span of each field to the span of the corresponding
-	// `max_encoded_len` call. This way, if one field's type doesn't implement
-	// `MaxEncodedLen`, the compiler's error message will underline which field


The old code was using the span of each field, aka what is written above. You say this did not worked?

zdave-parity · 2025-07-30T15:52:49Z

Looks good, but please add some test

Added a few tests.

gui1117 · 2025-08-02T04:52:53Z

Note that this is a breaking change (although I agree it is a fix).

This compiled:

diff --git a/tests/max_encoded_len.rs b/tests/max_encoded_len.rs
index 5a41a10..4a86285 100644
--- a/tests/max_encoded_len.rs
+++ b/tests/max_encoded_len.rs
@@ -307,3 +307,10 @@ fn skip_enum_struct_test() {
        let _ = Enum::<NoCodecType, NoCodecNoDefaultType>::A(NoCodecNoDefaultType);
        let _ = StructNamed::<NoCodecType> { a: NoCodecType, b: 0 }.a;
 }
+
+use parity_scale_codec::HasCompact;
+#[derive(Encode, MaxEncodedLen)]
+struct Foo<T: HasCompact + MaxEncodedLen> {
+       #[codec(compact)]
+       t: T,
+}

But now fails with:

error[E0277]: the trait bound `<T as HasCompact>::Type: MaxEncodedLen` is not satisfied
   --> tests/max_encoded_len.rs:312:18
    |
312 | #[derive(Encode, MaxEncodedLen)]
    |                  ^^^^^^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `<T as HasCompact>::Type`
    |
    = note: this error originates in the derive macro `MaxEncodedLen` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `<T as HasCompact>::Type: Encode` is not satisfied
   --> tests/max_encoded_len.rs:312:18
    |
312 | #[derive(Encode, MaxEncodedLen)]
    |                  ^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `<T as HasCompact>::Type`
    |
    = note: required for `<T as HasCompact>::Type` to implement `Encode`

I think polkadot-sdk or some ecosystem crates may fail to compile, we should check their compilation before releasing a minor version.
If polkadot-sdk and other crates compile, maybe it is ok.

* Bump proc-macro2 from 1.0.94 to 1.0.95 (paritytech#725) Bumps [proc-macro2](https://github.com/dtolnay/proc-macro2) from 1.0.94 to 1.0.95. - [Release notes](https://github.com/dtolnay/proc-macro2/releases) - [Commits](dtolnay/proc-macro2@1.0.94...1.0.95) --- updated-dependencies: - dependency-name: proc-macro2 dependency-version: 1.0.95 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix derive(Decode) for enums with lifetime parameters and struct-like variants (paritytech#726) * Fix derive(Decode) for enums with lifetime parameters and struct-like variants * FMT --------- Co-authored-by: Bastian Köcher <info@kchr.de> * Bump syn from 2.0.100 to 2.0.101 (paritytech#727) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.100 to 2.0.101. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@2.0.100...2.0.101) --- updated-dependencies: - dependency-name: syn dependency-version: 2.0.101 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump trybuild from 1.0.104 to 1.0.105 (paritytech#732) Bumps [trybuild](https://github.com/dtolnay/trybuild) from 1.0.104 to 1.0.105. - [Release notes](https://github.com/dtolnay/trybuild/releases) - [Commits](dtolnay/trybuild@1.0.104...1.0.105) --- updated-dependencies: - dependency-name: trybuild dependency-version: 1.0.105 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix `DecodeWithMemTracking` bounds for `Cow` (paritytech#735) * Fix `DecodeWithMemTracking` bounds for `Cow` The current bounds for `Cow` are incorrect and make `Cow` unusable with `DecodeWithMemTracking`. This is a breaking change, but as before it was not working no code should break by this. * FMT * Fix performance regression (paritytech#731) * Release 3.7.5 (paritytech#738) * Bump rustversion from 1.0.20 to 1.0.21 (paritytech#739) Bumps [rustversion](https://github.com/dtolnay/rustversion) from 1.0.20 to 1.0.21. - [Release notes](https://github.com/dtolnay/rustversion/releases) - [Commits](dtolnay/rustversion@1.0.20...1.0.21) --- updated-dependencies: - dependency-name: rustversion dependency-version: 1.0.21 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump proptest from 1.6.0 to 1.7.0 (paritytech#741) Bumps [proptest](https://github.com/proptest-rs/proptest) from 1.6.0 to 1.7.0. - [Release notes](https://github.com/proptest-rs/proptest/releases) - [Changelog](https://github.com/proptest-rs/proptest/blob/main/CHANGELOG.md) - [Commits](https://github.com/proptest-rs/proptest/commits) --- updated-dependencies: - dependency-name: proptest dependency-version: 1.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump syn from 2.0.101 to 2.0.103 (paritytech#744) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.101 to 2.0.103. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@2.0.101...2.0.103) --- updated-dependencies: - dependency-name: syn dependency-version: 2.0.103 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump syn from 2.0.103 to 2.0.104 (paritytech#746) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.103 to 2.0.104. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@2.0.103...2.0.104) --- updated-dependencies: - dependency-name: syn dependency-version: 2.0.104 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump enumflags2 from 0.7.11 to 0.7.12 (paritytech#742) Bumps [enumflags2](https://github.com/meithecatte/enumflags2) from 0.7.11 to 0.7.12. - [Release notes](https://github.com/meithecatte/enumflags2/releases) - [Commits](meithecatte/enumflags2@v0.7.11...v0.7.12) --- updated-dependencies: - dependency-name: enumflags2 dependency-version: 0.7.12 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix clippy warnings in derive macro (paritytech#747) (paritytech#748) Replace unsafe casts with format\! macro in field name generation. The old code was casting usize to u8 which triggered clippy warnings. Add test case to prevent regression. * Bump trybuild from 1.0.105 to 1.0.106 (paritytech#749) Bumps [trybuild](https://github.com/dtolnay/trybuild) from 1.0.105 to 1.0.106. - [Release notes](https://github.com/dtolnay/trybuild/releases) - [Commits](dtolnay/trybuild@1.0.105...1.0.106) --- updated-dependencies: - dependency-name: trybuild dependency-version: 1.0.106 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix missing target_has_atomic ptr (paritytech#751) * Bump proc-macro-crate from 3.1.0 to 3.3.0 (paritytech#752) Bumps [proc-macro-crate](https://github.com/bkchr/proc-macro-crate) from 3.1.0 to 3.3.0. - [Release notes](https://github.com/bkchr/proc-macro-crate/releases) - [Commits](https://github.com/bkchr/proc-macro-crate/commits/v3.3.0) --- updated-dependencies: - dependency-name: proc-macro-crate dependency-version: 3.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump trybuild from 1.0.106 to 1.0.110 (paritytech#756) Bumps [trybuild](https://github.com/dtolnay/trybuild) from 1.0.106 to 1.0.110. - [Release notes](https://github.com/dtolnay/trybuild/releases) - [Commits](dtolnay/trybuild@1.0.106...1.0.110) --- updated-dependencies: - dependency-name: trybuild dependency-version: 1.0.110 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix derive(MaxEncodedLen) with codec(compact/encoded_as) (paritytech#754) * Fix derive(MaxEncodedLen) with codec(compact/encoded_as) * UI test: MaxEncodedLen derive with NotMel field * UI test: MaxEncodedLen derive with encoded_as NotMel field * Trivial MaxEncodedLen derive + codec(compact/encoded_as) tests * Bump rustversion from 1.0.21 to 1.0.22 (paritytech#757) Bumps [rustversion](https://github.com/dtolnay/rustversion) from 1.0.21 to 1.0.22. - [Release notes](https://github.com/dtolnay/rustversion/releases) - [Commits](dtolnay/rustversion@1.0.21...1.0.22) --- updated-dependencies: - dependency-name: rustversion dependency-version: 1.0.22 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump actions/checkout from 4 to 5 (paritytech#758) Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump proc-macro2 from 1.0.95 to 1.0.97 (paritytech#760) Bumps [proc-macro2](https://github.com/dtolnay/proc-macro2) from 1.0.95 to 1.0.97. - [Commits](dtolnay/proc-macro2@1.0.95...1.0.97) --- updated-dependencies: - dependency-name: proc-macro2 dependency-version: 1.0.97 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump syn from 2.0.104 to 2.0.105 (paritytech#761) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.104 to 2.0.105. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@2.0.104...2.0.105) --- updated-dependencies: - dependency-name: syn dependency-version: 2.0.105 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump arbitrary from 1.4.1 to 1.4.2 (paritytech#762) Bumps [arbitrary](https://github.com/rust-fuzz/arbitrary) from 1.4.1 to 1.4.2. - [Changelog](https://github.com/rust-fuzz/arbitrary/blob/main/CHANGELOG.md) - [Commits](rust-fuzz/arbitrary@v1.4.1...v1.4.2) --- updated-dependencies: - dependency-name: arbitrary dependency-version: 1.4.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump criterion from 0.5.1 to 0.7.0 (paritytech#753) * Bump criterion from 0.5.1 to 0.7.0 Bumps [criterion](https://github.com/bheisler/criterion.rs) from 0.5.1 to 0.7.0. - [Changelog](https://github.com/bheisler/criterion.rs/blob/master/CHANGELOG.md) - [Commits](bheisler/criterion.rs@0.5.1...0.7.0) --- updated-dependencies: - dependency-name: criterion dependency-version: 0.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Fix build --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Serban Iorga <serban300@gmail.com> * parity_scale_codec -> jam_codec * Fix UI tests * Fix clippy warning * More parity_scale_codec -> jam_codec --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: ImplOfAnImpl <ImplOfAnImpl@users.noreply.github.com> Co-authored-by: Bastian Köcher <info@kchr.de> Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: Serban Iorga <serban300@gmail.com> Co-authored-by: Boris Onchev <frost_samuraj@hotmail.com>

Fix derive(MaxEncodedLen) with codec(compact/encoded_as)

b2da40b

zdave-parity commented Jul 28, 2025

View reviewed changes

bkchr reviewed Jul 29, 2025

View reviewed changes

zdave-parity added 3 commits July 30, 2025 16:48

UI test: MaxEncodedLen derive with NotMel field

f6429a9

UI test: MaxEncodedLen derive with encoded_as NotMel field

2d49ddb

Trivial MaxEncodedLen derive + codec(compact/encoded_as) tests

fa44701

zdave-parity force-pushed the dave/fix-max-encoded-len-with-attrs branch from 0189de3 to fa44701 Compare July 30, 2025 15:49

bkchr approved these changes Aug 1, 2025

View reviewed changes

bkchr merged commit 8f6ebec into master Aug 1, 2025
17 checks passed

zdave-parity deleted the dave/fix-max-encoded-len-with-attrs branch August 18, 2025 21:50

This was referenced Aug 18, 2025

Merge parity-scale-codec #763

Closed

Merge parity-scale-codec paritytech/jam-codec#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix derive(MaxEncodedLen) with codec(compact/encoded_as)#754

Fix derive(MaxEncodedLen) with codec(compact/encoded_as)#754
bkchr merged 4 commits intomasterfrom
dave/fix-max-encoded-len-with-attrs

zdave-parity commented Jul 28, 2025

Uh oh!

zdave-parity Jul 28, 2025

Uh oh!

zdave-parity Jul 28, 2025

Uh oh!

bkchr Jul 29, 2025

Uh oh!

zdave-parity Jul 29, 2025

Uh oh!

bkchr left a comment

Uh oh!

bkchr Jul 29, 2025

Uh oh!

zdave-parity commented Jul 30, 2025

Uh oh!

Uh oh!

gui1117 commented Aug 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zdave-parity commented Jul 28, 2025

Uh oh!

zdave-parity Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

zdave-parity Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

bkchr Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

zdave-parity Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

bkchr left a comment

Choose a reason for hiding this comment

Uh oh!

bkchr Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

zdave-parity commented Jul 30, 2025

Uh oh!

Uh oh!

gui1117 commented Aug 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants