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

Juvix to Isabelle/HOL translation #2752

Merged
merged 16 commits into from
Jun 5, 2024
Merged

Juvix to Isabelle/HOL translation #2752

merged 16 commits into from
Jun 5, 2024

Conversation

lukaszcz
Copy link
Collaborator

@lukaszcz lukaszcz commented Apr 22, 2024

  • Closes Translate Juvix types to Isabelle/HOL #2689
  • Adds the command juvix isabelle program.juvix which translates a given file to an Isabelle/HOL theory.
  • Currently, only a single module is translated.
  • By default translates types and function signatures. Translating function signatures can be disabled with the --only-types option.

Blocked by:

@lukaszcz lukaszcz added this to the 0.6.2 milestone Apr 22, 2024
@lukaszcz lukaszcz self-assigned this Apr 22, 2024
@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch 2 times, most recently from 2f415c3 to 7bb8ce5 Compare April 24, 2024 13:57
@lukaszcz lukaszcz marked this pull request as ready for review April 24, 2024 15:35
@lukaszcz lukaszcz requested review from jonaprieto, janmasrovira and paulcadman and removed request for jonaprieto and janmasrovira April 24, 2024 15:35
@jonaprieto jonaprieto added the enhancement New feature or request label Apr 25, 2024
@janmasrovira
Copy link
Collaborator

any ideas of how this could be tested?

Maybe we could have a parser for the subset of isabelle that we are producing. Then we would manually write the expected isabelle file and store it in the test suite, and then we would compare asts between the generated code and the parsed code. We could also compare the text if we don't wan't to write a parser, but that's a bit brittle.

@lukaszcz
Copy link
Collaborator Author

any ideas of how this could be tested?

Maybe we could have a parser for the subset of isabelle that we are producing. Then we would manually write the expected isabelle file and store it in the test suite, and then we would compare asts between the generated code and the parsed code. We could also compare the text if we don't wan't to write a parser, but that's a bit brittle.

That's a good question. I wanted to avoid both of these solutions (both require future maintenance) but I didn't have any better idea.

How do we test HTML and Markdown generation?

@jonaprieto
Copy link
Collaborator

jonaprieto commented Apr 25, 2024

How do we test HTML and Markdown generation?

For Markdown, we have tests in test/BackendMarkdown/ and smoke tests. For Html, as far as I remember, we only have smoke tests.

any ideas of how this could be tested?

Let's install Isabelle in the CI, and test with it that the generation produces valid theories.

janmasrovira
janmasrovira previously approved these changes Apr 26, 2024
Copy link
Collaborator

@jonaprieto jonaprieto left a comment

Choose a reason for hiding this comment

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

Wait for me, I need to test with some examples from the specs, before merging this.

@lukaszcz
Copy link
Collaborator Author

Wait for me, I need to test with some examples from the specs, before merging this.

It would be very helpful to learn what works and what still needs to be implemented/improved, but aside of obvious bugs I think any feature requests should be delegated to separate future PRs. So that we have an initial working version first, even if it's somewhat restricted in what it can handle.


instance PrettyCode Theory where
ppCode Theory {..} = do
n <- ppCode _theoryName
Copy link
Collaborator

Choose a reason for hiding this comment

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

The theory's name must correspond to the file's name. Currently, it's printing the qualified name of the Juvix module.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When is the generated file name different from the qualified named of the Juvix module? I think it's always the same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can theory names in Isabelle contain dots?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I got an error that says "Bad theory name .. with qualification" if we use dots.
However, I couldn't find any mention of this in the "Isabelle System Manual." So, they probably want the names of theories to be like regular names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, maybe then I just replace . with e.g. __ in both file and theory names.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That won't work either. Use the unqualified name of the module as the name of the theory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, can do that (potentially problematic in the future when translating multiple modules).

Copy link
Collaborator

@jonaprieto jonaprieto May 6, 2024

Choose a reason for hiding this comment

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

Indeed. But we can generate the Isabelle theories following the module's folder structure of the Juvix modules. We do this for HTML output. We can do that in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch from cbb224c to 638d946 Compare May 6, 2024 08:03
@lukaszcz
Copy link
Collaborator Author

lukaszcz commented May 7, 2024

For Markdown, we have tests in test/BackendMarkdown/ and smoke tests.

The single positive markdown test is very brittle because it just compares the output to a pre-stored file. I just had to regenerate it after changing some unrelated thing in the scoper, which changed the nameId numbers, which changed the generated markdown text.

@lukaszcz lukaszcz requested a review from jonaprieto May 7, 2024 16:59
@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch 2 times, most recently from e0b0666 to 093f9c9 Compare May 14, 2024 17:45
@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch from 093f9c9 to 6de024d Compare May 15, 2024 18:16
@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch from 6de024d to 4ceb57a Compare May 17, 2024 13:00
@lukaszcz lukaszcz force-pushed the juvix-to-isabelle branch from 4ceb57a to da8f71f Compare May 22, 2024 13:57
@jonaprieto jonaprieto merged commit ce938ef into main Jun 5, 2024
4 checks passed
@jonaprieto jonaprieto deleted the juvix-to-isabelle branch June 5, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:isabelle enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Translate Juvix types to Isabelle/HOL
3 participants