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: box ParserErrors in InterpreterError #7373

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

InterpreterError is currently 488 bytes (and so all of IResults are as well). Most of that size comes from it holding a ParserError inside one of its variants so I've boxed that which reduces the size of an InterpreterError to 192 bytes.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

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

@TomAFrench TomAFrench requested a review from a team February 14, 2025 12:10
Copy link
Collaborator

@asterite asterite 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!

Is this noticeable somewhere? I'm asking mainly because I'm thinking of reviving #7147 : it's probably not noticeable in a big compilation, but it is faster (milliseconds faster) and that always helps if it's seen from an LSP perspective (where every millisecond matters).

@asterite
Copy link
Collaborator

That said... Value is already 888 bytes. So IResult's size doesn't change.

@TomAFrench
Copy link
Member Author

Value is already 888 bytes. So IResult's size doesn't change.

Oof I didn't see that. Will make a followup

@TomAFrench TomAFrench added this pull request to the merge queue Feb 14, 2025
Merged via the queue into master with commit 2b6db07 Feb 14, 2025
103 checks passed
@TomAFrench TomAFrench deleted the tf/box-parser-errors branch February 14, 2025 15:22
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 14, 2025
…oir-lang/noir#7393)

chore: document traits required to be in scope (noir-lang/noir#7387)
fix: field zero division in brillig (noir-lang/noir#7386)
chore: box `ParserError`s in `InterpreterError` (noir-lang/noir#7373)
chore: remove unnecessary dereferencing within brillig vm (noir-lang/noir#7375)
fix: give "correct" error when trying to use AsTraitPath (noir-lang/noir#7360)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 14, 2025
…oir#7393)

chore: document traits required to be in scope (noir-lang/noir#7387)
fix: field zero division in brillig (noir-lang/noir#7386)
chore: box `ParserError`s in `InterpreterError` (noir-lang/noir#7373)
chore: remove unnecessary dereferencing within brillig vm (noir-lang/noir#7375)
fix: give "correct" error when trying to use AsTraitPath (noir-lang/noir#7360)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 14, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(ssa): Do not deduplicate division by a zero constant
(noir-lang/noir#7393)
chore: document traits required to be in scope
(noir-lang/noir#7387)
fix: field zero division in brillig
(noir-lang/noir#7386)
chore: box `ParserError`s in `InterpreterError`
(noir-lang/noir#7373)
chore: remove unnecessary dereferencing within brillig vm
(noir-lang/noir#7375)
fix: give "correct" error when trying to use AsTraitPath
(noir-lang/noir#7360)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <[email protected]>
TomAFrench added a commit that referenced this pull request Feb 14, 2025
…rom-brillig

* master:
  chore: allow opting in to displaying benchmark comments (#7399)
  chore: box `ExprValue` in `Value` enum (#7388)
  chore: pull out refactored methods from u128 branch (#7385)
  feat: require safety comments instead of safety doc comments (#7295)
  fix(ssa): Do not deduplicate division by a zero constant (#7393)
  chore: document traits required to be in scope (#7387)
  fix: field zero division in brillig (#7386)
  chore: box `ParserError`s in `InterpreterError` (#7373)
  chore: remove unnecessary dereferencing within brillig vm (#7375)
  fix: give "correct" error when trying to use AsTraitPath (#7360)
  chore: avoid u128s in brillig memory (#7363)
  chore: update docs about integer overflows (#7370)
  fix!: Only decrement the counter of an array if its address has not changed (#7297)
  fix: let LSP read `noirfmt.toml` for formatting files (#7355)
  chore: deprecate keccak256 (#7361)
  feat: `FunctionDefinition::as_typed_expr` (#7358)
  feat(performance): Check sub operations against induction variables (#7356)
  chore: avoid doing all brillig integer arithmetic on u128s (#7357)
TomAFrench added a commit that referenced this pull request Feb 14, 2025
* master:
  chore: allow opting in to displaying benchmark comments (#7399)
  chore: box `ExprValue` in `Value` enum (#7388)
  chore: pull out refactored methods from u128 branch (#7385)
  feat: require safety comments instead of safety doc comments (#7295)
  fix(ssa): Do not deduplicate division by a zero constant (#7393)
  chore: document traits required to be in scope (#7387)
  fix: field zero division in brillig (#7386)
  chore: box `ParserError`s in `InterpreterError` (#7373)
  chore: remove unnecessary dereferencing within brillig vm (#7375)
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