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

fix(engine) Make sub-parts of Quote visited by visitors #1206

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

maximebuyse
Copy link
Contributor

Fixes #1196

The investigation for #1196 revealed that subparts of Quote variant in the AST were ignored by visitors. The easiest way to change that is to use a quote_contents ADT inside instead of an inlined polymorphic variants. This involves quite a few changes here and there.

@maximebuyse
Copy link
Contributor Author

I succeeded in making a version that fixes the bug for F*. But I am not sure that it does the right thing for the generic printer (at least it builds correctly and doesn't break tests).

Copy link
Contributor

@karthikbhargavan karthikbhargavan left a comment

Choose a reason for hiding this comment

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

Generally, this looks good to me. Let's test on libcrux, and maybe you can resolve some of my comments.

Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

Thanks, there's a few things to change, but that looks good :)

@maximebuyse maximebuyse requested a review from W95Psp January 6, 2025 10:32
@maximebuyse
Copy link
Contributor Author

Thanks, there's a few things to change, but that looks good :)

Thanks, everything should be good now!

Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

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

LGTM, let's merge!

@maximebuyse maximebuyse added this pull request to the merge queue Jan 6, 2025
Merged via the queue into main with commit ffcd803 Jan 6, 2025
14 of 15 checks passed
@maximebuyse maximebuyse deleted the fix-visitors-quote branch January 6, 2025 11:20
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.

No rewriting of $ names in fstar! macro with bundling
3 participants