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

Add syntactic to whitelist #4597

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Add syntactic to whitelist #4597

merged 2 commits into from
Aug 22, 2024

Conversation

Scott-Guest
Copy link
Contributor

Part of #4579

Adds the syntactic attribute to the whitelist, and checks that it is only applied to simplification rules.

Notably, we still need to:

  • Check that the arguments actually refer to existing clauses
  • Tag the underlying clauses and re-construct the attribute at the end of the compilation pipeline because the requires clauses may be manipulated during compilation

@PetarMax
Copy link
Contributor

@Scott-Guest Were you meaning to add @geo2a and myself as reviewers?

Copy link
Contributor

@geo2a geo2a left a comment

Choose a reason for hiding this comment

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

LGTM!

Just curios: does the frontend has any unit tests for this sort of functionality?

@Scott-Guest Scott-Guest marked this pull request as ready for review August 22, 2024 03:03
@Scott-Guest
Copy link
Contributor Author

Scott-Guest commented Aug 22, 2024

@geo2a We have tests for the attribute whitelist infrastructure in general (buried somewhere in regression-new), but don't specifically test each entry. I'll add a small test for the "only on simplification rules" part though.

@rv-jenkins rv-jenkins merged commit 6aae149 into develop Aug 22, 2024
17 checks passed
@rv-jenkins rv-jenkins deleted the syntactic branch August 22, 2024 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants