Cleanup and new tests #87
Conversation
Cppo_eval.ml: make the type [env] an algebraic data type. This avoids some confusion, as the constructors [`Def] and [`Defun] were previously used in two different types, namely [node] and [env], with different arguments.
Here, [args] is never empty, so [n] is never zero, so this test is useless.
In the definition of [`Ident] in the type [node], the third parameter used to have type [actuals option]: it was either [None] or [Some args] where [args] was a nonempty list of actuals. It is simpler to let this parameter have type [actuals]. There is no need for an option.
The file test/dune is modified to use more modern rule syntax. This changes the generated .opam files. The purpose of this change is to allow negative tests. `with-accepted-exit-codes` requires dune 2.0.
New positive tests: lexical.cppo New negative tests: arity_mismatch.cppo applied_to_none.cppo expects_no_args.cppo already_defined.cppo at_least_one_arg.cppo
There was no strong reason for this distinction to exist. A few new auxiliary functions are isolated: [bind_one], [bind_many]. A few error messages change slightly (they become more uniform). The expected test output is adjusted in a separate commit.
There was no strong reason for this distinction to exist.
| @@ -15,7 +15,7 @@ | |||
| (name cppo) | |||
| (depends | |||
| (ocaml (>= 4.02.3)) | |||
| (dune (>= 1.10)) | |||
| (dune (>= 2.0)) | |||
There was a problem hiding this comment.
Maybe worth making dune a build-only dependency?
Also, should we change the depends field for cppo_ocamlbuild as well?
There was a problem hiding this comment.
Making dune a build-only dependency: this makes sense, but I don't know how to do it. In the dune-project file, I can write either (dune (>= 2.0)) or (dune :build) but I am unable to combine these two declarations into a single one; dune rejects everything I try.
There was a problem hiding this comment.
Changing the depends field cppo_ocamlbuild seems preferable; I will add a commit to do it.
Maybe this is not necessary, but it seems safer.
|
If everyone is good with it, I will merge the request. BTW, I propose that @fpottier be given commit access to ocaml-community unless there are objections. |
|
Yes we can merge now, and fix the CI in #88. |
Write access to this repo now granted. |
|
Thanks! |
This PR proposes a cleanup of some parts of the code, especially in
Cppo_eval. Internally, the distinction between ordinary macros and parameterized macros disappears; all macros are parameterized, but can have 0 parameters.The observable behavior of
cppois unchanged, except that some error messages change (this is visible in the.reffiles associated with the new negative tests).This PR also adds several new tests, including positive tests (which are expected to succeed) and negative tests (which are expected to fail).
The negative tests use
with-accepted-exit-codes, which requiresdune2.0. If this is not acceptable, I could remove this part of the PR.My motivation for creating this PR is that I wanted to add support for higher-order macros. I have started investigating this extension, but it is not ready yet.