Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

feat(brillig): Allow dynamic-size foreign calls #370

Merged
merged 23 commits into from
Jun 16, 2023

Conversation

ludamad
Copy link
Contributor

@ludamad ludamad commented Jun 13, 2023

Description

Problem*

Incremental progress towards noir-lang/noir#1556

Takes this milestone "Add support for slices such that in Brillig we can call an oracle and have it return a slice with length not known and enumerate over that slice" and does the part necessary for such oracles in brillig vm

As an intro, start with the comment in brillig_vm/src/opcodes.rs

Additional Context

PR Checklist*

  • [*] I have tested the changes locally.
  • [*] I have formatted the changes with Prettier and/or cargo fmt on default settings.

@ludamad ludamad changed the title Adam/foreign call vectors chore(brillig): Allow dynamic-size foreign calls Jun 13, 2023
@ludamad ludamad marked this pull request as ready for review June 13, 2023 17:53
brillig_vm/src/lib.rs Outdated Show resolved Hide resolved
@ludamad
Copy link
Contributor Author

ludamad commented Jun 14, 2023

Very weird thing - when I closed my unconventionally-titled PR (had a DONT MERGE at start), this one started to report that failure

brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/opcodes.rs Outdated Show resolved Hide resolved
brillig_vm/src/lib.rs Outdated Show resolved Hide resolved
brillig_vm/src/lib.rs Outdated Show resolved Hide resolved
brillig_vm/src/lib.rs Outdated Show resolved Hide resolved
@vezenovm
Copy link
Contributor

Mostly nit comments but this looks solid

@ludamad ludamad enabled auto-merge June 14, 2023 12:00
@ludamad
Copy link
Contributor Author

ludamad commented Jun 14, 2023

@vezenovm I merged your work into here, this could use another pass

brillig_vm/src/lib.rs Outdated Show resolved Hide resolved
@TomAFrench TomAFrench changed the title chore(brillig): Allow dynamic-size foreign calls feat(brillig): Allow dynamic-size foreign calls Jun 14, 2023
@ludamad ludamad closed this in #375 Jun 14, 2023
auto-merge was automatically disabled June 14, 2023 14:43

Pull request was closed

@ludamad ludamad reopened this Jun 14, 2023
Copy link
Contributor

@vezenovm vezenovm left a comment

Choose a reason for hiding this comment

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

@ludamad I resolved the merge conflicts w/ master after switching the way handle failure's for invalid foreign call results. I'm going to approve and you take a pass on my latest commits this PR looks good to me.

@vezenovm vezenovm requested a review from kevaundray June 16, 2023 17:16
@kevaundray kevaundray added this pull request to the merge queue Jun 16, 2023
Merged via the queue into master with commit 5ba0349 Jun 16, 2023
@kevaundray kevaundray mentioned this pull request Jun 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants