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

fix(brillig_gen): Pass correct size of complex types input for brillig foreign calls #1922

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

vezenovm
Copy link
Contributor

Description

We were passing the size of an array from Type::Array directly when generating the inputs for a brillig foreign call. This wouldn't break for normal arrays or singular structs, but breaks in the case of an array of structs. For example if we have an array of two structs with two fields each, only the first two fields would be passed to the foreign call as the entire array is of size 2.

Problem*

Quick issue found while doing logging PR

Summary*

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
    I held off on adding a struct test directly as my println PR will have much more comprehensive tests that deserialize arrays of structs.

  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@vezenovm vezenovm requested a review from jfecher July 13, 2023 14:44
@vezenovm vezenovm changed the title fix(brillig_gen): Pass correct size of HeapArray passed to brillig foreign calls for complex types fix(brillig_gen): Pass correct size of complex types input for brillig foreign calls Jul 13, 2023
@kevaundray kevaundray added this pull request to the merge queue Jul 13, 2023
Merged via the queue into master with commit 04c89d2 Jul 13, 2023
@kevaundray kevaundray deleted the mv/brillig-ff-struct-input-fix branch July 13, 2023 15:52
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.

2 participants