-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat: Typings generator with generics #6235
Conversation
…rotocol/aztec-packages into arv/generics_in_protocol_circuits
* Since monomorphization of the generics destroys information, this process is not guaranteed to return the original structure. | ||
* However, it should succesfully unify all struct types that share the same name and field names. | ||
*/ | ||
export class Demonomorphizer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw a lots of blue squiggles, apparently the spell checking plugin doesn't think this is a read word 😂 Might have to add all the variations to cspell.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
if (structTypesToExport.has(id)) { | ||
continue; | ||
} | ||
structTypesToExport.set(id, struct.structType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't it break for structs like these?
struct Foo {
value: Field,
}
struct Foo {
value: Bar,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the types used are passed through the demonomorphizer, it guarantees that it'll only generate one struct type for all struct variants with the same id. In this case, since these two structs have the same name and field names, (same id) it'll demonomorphize them into
struct Foo<A> {
value: A
}
So the generate_ts_from_abi can safely generate one type for each id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant work! 🎉
* master: (25 commits) fix: Enable client proof tests (#6249) chore: update cspell for abi demonomorphizer (#6258) feat(aztec-nr): add 'with_gas()' function to avm call interface (#6256) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg fix: Pw/update merge check (#6201) chore(master): Release 0.37.1 (#6148) fix: Cl/split out e2e tests (#6242) feat: Typings generator with generics (#6235) chore(ci): fix restarts with fresh spot, acir test fixes, non-mandatory benches (#6226) chore: misc AVM migration prep changes (#6253) feat!: AES blackbox (#6016) chore(docs): Fix some typos in specs of private kernel initial (#6224) chore(aztec-macros): avm function return types are auto tagged as `pub` (#6250) chore(aztec-nr): create a 'with_selector' version of `emit_unencrypted_log` in avm context (#6248) fix: registering PublicDataWitness in JsonRpcServer (#6243) feat: Sync from noir (#6234) feat(avm-simulator): consider previous pending nullifiers across enqueued calls (#6188) ...
This PR adds a demonomorphizer in the
generate_ts_from_abi
script/tool so we can use generic types in the protocol circuits interface. It also fixes the MembershipWitness issue where we had to use concrete types instead of a generic due to the old limitation of the types generator.