Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substitute BTreeMap/BTreeSet generated types for Vec #459

Merged
merged 5 commits into from
Feb 21, 2022

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Feb 21, 2022

Recently, we hit an issue (#454) where a key type given to a BTreeMap did not impl Ord, leading to compile errors attempting to use the generated code.

This PR impls @ascjones's suggestion in #456 and makes types that would have been decoded into BTreeMap/BTreeSet instead decode to Vec<(K,V)> or Vec<K>. Since Vecs don't require their contents to impl any particular traits, this avoids such issues, while still preserving the decode order.

Closes #456 and #315

@jsdw jsdw requested review from ascjones and a team February 21, 2022 11:50
Copy link
Contributor

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

That looks easier than I thought! I think this is worth documenting somewhere, at least in the codegen module docs.

@jsdw
Copy link
Collaborator Author

jsdw commented Feb 21, 2022

That looks easier than I thought! I think this is worth documenting somewhere, at least in the codegen module docs.

Good point! And yup if I thought it would be this straightforward I would have done it first and avoided the previous hack!

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

🌮

@jsdw jsdw merged commit d01fdd7 into master Feb 21, 2022
@jsdw jsdw deleted the jsdw-vec-substitute branch February 21, 2022 14:18
icodezjb pushed a commit to chainx-org/substrate-subxt that referenced this pull request Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decode BTreeMap/HashMap etc as Vec<(K,V)>
3 participants