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: Rework CrateGraph to only have one root crate #2391

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Aug 21, 2023

Description

Problem*

Towards #2238

Summary*

This reworks the CrateGraph to only have one root crate. This allows us to lookup the root crate when building an ABI for it. I also cleaned up the Dummy case so we aren't storing a std::usize::MAX for each dummy id.

This significantly cleans up my prototype solution in #2374

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.

@phated phated requested a review from kevaundray August 22, 2023 15:29
kevaundray
kevaundray previously approved these changes Aug 22, 2023
Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

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

Looks good - left comment re the question in the code

@kevaundray
Copy link
Contributor

RE, Results versus panicking, I think we can create another issue to return results here and panic further up the stack or possibly not panic and keep bubbling it up the stack -- We were already panicking which is why I think its okay to leave that for another PR

@kevaundray kevaundray added this pull request to the merge queue Aug 22, 2023
Merged via the queue into master with commit 707685c Aug 22, 2023
@kevaundray kevaundray deleted the phated/root-crate-variant branch August 22, 2023 22:19
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.

2 participants