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

Circom grammar update #511

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Circom grammar update #511

wants to merge 2 commits into from

Conversation

Frodan
Copy link
Contributor

@Frodan Frodan commented Sep 18, 2024

Updating tree-sitter and semgrep grammar for circom language.

@Frodan Frodan requested a review from a team as a code owner September 18, 2024 15:31
@Frodan Frodan requested a review from brandonspark September 18, 2024 15:31
@aryx aryx requested a review from mjambon September 18, 2024 16:23
@aryx
Copy link
Collaborator

aryx commented Sep 18, 2024

@mjambon is in the middle of a big refactoring in ocaml-tree-sitter-semgrep but once his PR is merged we can probably merge this one.

@aryx
Copy link
Collaborator

aryx commented Sep 23, 2024

@mjambon can we merge this? Are you done with your tree-sitter version refactoring?

@Frodan
Copy link
Contributor Author

Frodan commented Sep 30, 2024

Hi, any updates on merging this?

Copy link
Collaborator

@aryx aryx left a comment

Choose a reason for hiding this comment

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

Sorry for the late review, we had an offsite last week.
Can you add some test corpus for those new supported constructs?
Also was it possible instead to modify the original grammar so that
we don't have to copy so much from main_component_definition and component_declaration?
like if you introduce an intermediate rule for what is after the =, like main_component_value, then it's possible to override just that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants