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

chore: Decouple noirc_abi from frontend by introducing PrintableType #2373

Merged
merged 9 commits into from
Aug 23, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Aug 18, 2023

Description

Problem*

Towards #2238

Summary*

This completes the decoupling of the noirc_abi from the noirc_frontend by introducing a new crate called noirc_printable_type. Right now, the PrintableType is similar to the AbiType but is a separate implementation to allow these to grow independently. For example, we probably want to print tuples but might not want to support them in the ABI, or the more concrete example of #2238.

In doing this work, I also noticed room for improvement in how Display was implemented for PrintableType to avoid round-tripping to serde_json. Here's the new output from running nargo execute on https://github.com/noir-lang/noir/blob/master/crates/nargo_cli/tests/execution_success/debug_logs/src/main.nr:

"i: {i}, j: {j}"
i: 0x01, j: 0x02
i: 0x01, j: 0x02
i: 0x01, j: 0x02
myStruct { y: 0x01, x: 0x02 }
randomstring0x010x01
i: 0x01, s: myStruct { y: 0x01, x: 0x02 }
0x01
[0x01, 0x02]
s: myStruct { y: 0x01, x: 0x02 }, foo: fooStruct { my_struct: myStruct { y: 0x02, x: 0x01 }, foo: 0x02 }
x: 0, y: 1
s1: myStruct { y: 0x01, x: 0x02 }, s2: myStruct { y: 0x02, x: 0x1e }
foo1: fooStruct { my_struct: myStruct { y: 0x01, x: 0x02 }, foo: 0x0f }, foo2: fooStruct { my_struct: myStruct { y: 0x02, x: 0x0f }, foo: 0x1e }
fooStruct { my_struct: myStruct { y: 0x01, x: 0x02 }, foo: 0x0f }

I'm happy to make any println formatting changes here if we don't like this output.

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 have formatted the changes with Prettier and/or cargo fmt on default settings.

Base automatically changed from phated/remove-function-signature-from-abi to master August 21, 2023 13:36
@phated phated force-pushed the phated/separate-abi-and-print branch from 9f769ad to 1f76e23 Compare August 21, 2023 17:29
@phated phated marked this pull request as ready for review August 21, 2023 17:29
@phated phated requested a review from kevaundray August 21, 2023 22:03
crates/noirc_abi/src/lib.rs Outdated Show resolved Hide resolved
crates/noirc_abi/src/lib.rs Show resolved Hide resolved
crates/noirc_abi/src/lib.rs Show resolved Hide resolved
crates/noirc_frontend/src/monomorphization/mod.rs Outdated Show resolved Hide resolved
crates/noirc_printable_type/src/lib.rs Outdated Show resolved Hide resolved
crates/noirc_printable_type/src/lib.rs Outdated Show resolved Hide resolved
crates/noirc_printable_type/src/lib.rs Outdated Show resolved Hide resolved
crates/noirc_printable_type/src/lib.rs Outdated Show resolved Hide resolved
phated and others added 2 commits August 22, 2023 08:23
@phated phated requested a review from vezenovm August 22, 2023 22:46
crates/nargo/src/ops/foreign_calls.rs Show resolved Hide resolved
crates/noirc_printable_type/src/lib.rs Show resolved Hide resolved
@phated phated added this pull request to the merge queue Aug 23, 2023
Merged via the queue into master with commit 182d017 Aug 23, 2023
16 checks passed
@phated phated deleted the phated/separate-abi-and-print branch August 23, 2023 15:57
TomAFrench added a commit that referenced this pull request Aug 23, 2023
* master: (34 commits)
  chore: Decouple `noirc_abi` from frontend by introducing `PrintableType` (#2373)
  feat(brillig): Added locations for brillig artifacts (#2415)
  feat: Report compilation warnings before errors (#2398)
  chore: Rework `CrateGraph` to only have one root crate (#2391)
  chore: clippy fix (#2408)
  chore(deps): bump rustls-webpki from 0.101.1 to 0.101.4 (#2404)
  fix(acir): Attach locations to MemoryOps in ACIR (#2389)
  feat: Use equivalence information from equality assertions to simplify circuit (#2378)
  chore: fix body expr span (#2402)
  feat(attributes): enable custom attributes (#2395)
  chore: Remove `serde` from `noirc_frontend` (#2390)
  chore: allow parenthesizing in two type locations  (#2388)
  chore(ci): automatically delete cache entries associated with closed PRs (#2342)
  feat: Perform more checks for compile-time arithmetic (#2380)
  chore: Remove `noirc_abi::FunctionSignature` and define in terms of HIR (#2372)
  feat: Update to `acvm` 0.22.0 (#2363)
  chore: Update committed ACIR artifacts (#2376)
  feat(ssa): Merge slices in if statements with witness conditions (#2347)
  chore: Separate frontend `Visibility` and `Distinctness` from the ABI (#2369)
  feat: add syntax for specifying function type environments (#2357)
  ...
TomAFrench added a commit that referenced this pull request Aug 23, 2023
* master:
  chore: Decouple `noirc_abi` from frontend by introducing `PrintableType` (#2373)
  feat(brillig): Added locations for brillig artifacts (#2415)
  feat: Report compilation warnings before errors (#2398)
  chore: Rework `CrateGraph` to only have one root crate (#2391)
  chore: clippy fix (#2408)
  chore(deps): bump rustls-webpki from 0.101.1 to 0.101.4 (#2404)
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.

3 participants