-
Notifications
You must be signed in to change notification settings - Fork 18
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
Printer for terms #14
Conversation
@Gbury In the |
That's a typo, should indeed be |
It's corrected then. |
Should we create a new package, including the external dependencies in the dolmen.opam file, or compile dolmen.export only if the dependencies are available? |
Separate package, by creating a dolmen-export.opam file with the correct deps (i.e. |
@Gbury It's right. I forget it. |
src/export/smtlib.ml
Outdated
open Term | ||
|
||
(* auxiliar functions *) | ||
let pp_builtins = function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe reserve the pp_*
names for functions that actually print things (and thus have a type Format.formatter -> 'a -> unit
). Maybe use a name like pretty_builtins
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It's that the idea.
Should we treat this cases like that ?
In smtlib doesn't make sense to print a definition that is not a function nor a sort. |
Smtlib's |
@@ -1,71 +1,86 @@ | |||
(* This module contains the pretty-printer functions for smtlib *) | |||
|
|||
open Dolmen_std | |||
|
|||
open Pretty | |||
open Term |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really, really, really not a fan of using a lot of opens. Dolmen_std
is fine to open, but Pretty
and Term
should not be opened.
I don't think you've understood the design of the Pretty module (though that's probably my fault for not documenting it enough). The point of a So in a typical control flow, you'll have a function |
So, what to do with the other binders? |
Well, clearly, in a lot of cases (for instance, when a |
Yes. I will do it incrementally, first I expect to pretty print all the parsed smtlib files (some that I have generated), then we can do translations from other constructions available for instance in tptp. |
@Gbury Do you have any reason for implementing the order of
instead of
|
Typically, most type annotations are of the form: |
Alright. |
Should we create a new bash script for testing dolmen-exports? Which testing mechanism have you in mind? |
Basically, the goal of tests would be to check that parsing and printing are invers of one another, which can be divided into two distinct tests:
The two tests can be done at the same time, using a small ocaml program which does the following:
This would then need to be run on pretty much the whole smtlib to check that there are no errors. |
Additionally, it could be nice to have some quickcheck-like testing (using the same method as described in the previous comment, but using a randomly-generated AST instead of an AST coming from an input file). |
@Gbury What is the plan for the printing feature? |
The printing feature is the next big item on my todo list for dolmen, on which i'll be working once #84 is merged. My current plan is to add printing functors, which will take a view on terms, and return printing functions. I still have a few details concerning scopes[1] and differences in spec between languages[2] to work out, but it shouldn't delay things too much. When I have working code, I'll probably open a different PR (and close this one). As for the timeline, maybe I'll get enough time to get some working code around the end of October ? [1] : the printing function will take as first argument an environment, which will carry both a global and local state in order to accurately track scopes and potential renaming of variables/constants in order to avoid unintended shadowing. [2] : the module taken as argument to the printing functor will expect a view functions on terms. For simplicity, this view type will be defined by Dolmen and most likely use the current types builtins. The printing of builtins is straightforward when the same language is used as input and output, but for the other cases, it can be more tricky. Basically, there are a few subtle situations:
|
This is now a bit too outdated, so i'll close this PR and open a new one when I have the time to work on printing/exporting. |
Should close #8 once merged.
TODO: