Switch from HashSet to BTreeSet for reproducibility - #257
Open
sietseringers wants to merge 3 commits into
Open
Conversation
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
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.
This MR makes macro expansion reproducible, to facilitate reproducible builds.
Previous behaviour
#[nutype(...)]collected derive traits into aHashSet, whose iteration order is random. The result of this was that the order of#[derive(...)]entries and of the generatedimplblocks in the emitted code differed every time the macro ran. For downstream crates, this means that they cannot have reproducible builds.Fix
Traits now travel through a
BTreeSetinstead of aHashSet, and the five*DeriveTraitenums (any,decimal,float,integer,string) derivePartialOrd, Ord.The derived
Ordranks variants by declaration order, so the emitted order is now the order the variants are written in.The
HashSets that remain (common/validate.rs,float/validate.rs) are used only forcontainsand the boolean result ofinsert. They are never iterated, so they cannot leak ordering into the output or into which error gets reported first.Testing
nutype_macros/src/expansion_tests.rsexpands the same input 100 times and asserts every result is byte-identical, with one test per code path: string, integer, float, any, decimal, and conditional (cfg_attr) derives. The conditional case has its own test becauseprocess_conditional_derivesbuilds its sets separately from the unconditional path.string/generate/traits/mod.rsalso gained a unit test pinning the split of traits into transparent vs. irregular groups to declaration order, using an input deliberately built in neither declaration nor alphabetical order.Note
Making this testable required one production change:
parse_metanow callssyn::parse2instead ofsyn::parse(ts.into()).syn::parseis defined asparse2preceded by aproc_macro2 -> proc_macro -> proc_macro2round-trip, which panics outside a real proc-macro invocation. Consequentially,expand_nutypecould not be called from a unit test. The change drops the round-trip; the parsing call underneath is the same one.