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

feat: add --skip-prelude hidden CLI option #5033

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

michaeljklein
Copy link
Contributor

Description

Problem*

Usually, when debugging programs, they can be converted to a noirc_frontend/tests.rs test with or without including the stdlib/prelude.

However, workspace and multi-file-specific cases can be tedious to debug when the failing file has ~10 LOC and most of the context / parsing / type checking / etc. is all stdlib.

Summary*

Adds --skip-prelude flag to skip calling inject_prelude when collecting definitions.

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.

@@ -250,12 +254,14 @@ pub fn check_crate(
deny_warnings: bool,
disable_macros: bool,
use_elaborator: bool,
skip_prelude: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

It would probably be good to start grouping these together. It looks like in a lot of tests we just have four false variables being passed through.

Maybe we could put these options on the Context to easily thread them to other passes as well. This is non-blocking but just a thought

@michaeljklein michaeljklein marked this pull request as draft May 17, 2024 01:15
@vezenovm
Copy link
Contributor

@michaeljklein I see you marked this as a draft? Is this to add a Context struct? I think this can be done in a follow-up and we should get this in as it is beneficial for debugging

@jfecher
Copy link
Contributor

jfecher commented May 23, 2024

If this is ready I'd like it as well for testing the elaborator

Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM

@TomAFrench
Copy link
Member

Michael's out today so I'm going to merge this.

@TomAFrench TomAFrench marked this pull request as ready for review May 24, 2024 14:36
@TomAFrench TomAFrench enabled auto-merge May 24, 2024 14:36
@TomAFrench TomAFrench changed the title feat: add --skip-prelude hidden CLI option, skip inject_prelude when true feat: add --skip-prelude hidden CLI option May 24, 2024
auto-merge was automatically disabled May 27, 2024 13:14

Merge queue setting changed

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.

None yet

4 participants