Add an IsEmpty derive macro and use it in pos-accounting#2096
Open
oliv3rdrt wants to merge 1 commit into
Open
Conversation
PoSAccountingData had a hand-written is_empty that ANDs all of its fields, with a TODO asking for it to be generated. It is easy to add a field and forget to update such a method. Add an is-empty crate with an IsEmpty trait and a matching derive, mirroring the existing typename/typename-derive pair. The derive implements is_empty as the conjunction of each field's own is_empty, so a value is empty when all of its fields are empty. Apply it to PoSAccountingData and drop the manual method; pos-accounting re-exports the trait so existing callers keep working.
oliv3rdrt
requested review from
ImplOfAnImpl,
anyxem and
erubboli
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PoSAccountingData::is_emptyis written by hand as the conjunction of its five fields'is_emptycalls, and carries a// TODO: avoid manual implementation (mintlayer/mintlayer-core#669). As the issue notes, adding a field and forgetting to update such a method is an easy mistake.Change
Add an
IsEmptytrait and a derive macro for it, as a newis-empty/is-empty-derivecrate pair that mirrors the existingtypename/typename-derivesetup. The derive generatesis_emptyas the AND of each field's ownis_empty, so a struct is empty exactly when all of its fields are empty (a field-less struct is always empty). Every field type therefore needs anis_emptymethod: the standard collections,String, or anotherIsEmptytype.Apply the derive to
PoSAccountingData, removing the manual implementation and the TODO.pos-accountingre-exports the trait so the existing callers only need it in scope.Tests
The
is-emptycrate has unit tests covering named-field, tuple and unit structs, and the empty and non-empty cases. The existingpos-accountingandchainstatetests still pass.Closes #669