feat(lltz michelson): tests added

[alanmarkoTrilitech]

Context

https://app.asana.com/0/1207308081067689/1208120654364721

Description

This PR adds inline tests testing execution of each instruction. The purpose is to be able to see the output of each instruction and see what changes if any further PRs make. It additionally change the form of DSL into a more readable format.

Manual testing

dune build
dune runtest

[rinderknecht]

I have a general question. The optional parameter range has a default, dummy value, and, because of that, at least one following parameter cannot be labelled. This creates disparate declarations, including spurious () parameters. Do you plan to get rid of the optional parameter at one point?

[alanmarkoTrilitech]

I have a general question. The optional parameter range has a default, dummy value, and, because of that, at least one following parameter cannot be labelled. This creates disparate declarations, including spurious () parameters. Do you plan to get rid of the optional parameter at one point?

I was thinking about keeping it as is, because I'm not sure how could it be solved otherwse. It seems to me that the DSL looks reasonably good with one unlabelled param and all other labelled - e.g. let_in (var) ~rhs:...~in_.... For e.g. unit () and similar, it could be improved and i agree that it would be better without having to add the brackets, however having the optional range parameter seems more important at this point.

[rinderknecht]

I guess my question then boils down to: Do you need the optional parameter in the long term?

[alanmarkoTrilitech]

I guess my question then boils down to: Do you need the optional parameter in the long term?

well the optional parameter was suggested by Alistair I assume primarily for possibility to add better testing of the DSL. Furthermore sometimes, DSL could be used for easier compilation in some cases (e.g. writing more complex code first in DSL, then compiling). So, I imagine it won't have very significant use cases, but it's nice to have so that if I decide to use it I don't have to restructure the DSL again.

[rinderknecht]

My experience with LIGO is that injecting dummies for source location is a temptation that regularly led to error messages without source locations. Like exceptions, they are convenient but often make us look bad. If we use a default parameter with a dummy, all it takes is to forget to pass a location and we have this issue.

I'd rather have a normal version of the instruction set with mandatory locations (ranges) and a specialised version for testing only, with partial evaluation of those to dummies.

Now, to rephrase my issue with the current design (with optionals): we have a fairly large instruction set, so we need a fairly uniform calling convention. For example, you changed

let let_mut_in ?(range = dummy) (var, rhs, in_) = create ~range (LLTZ.E.Let_mut_in { let_var = var; rhs; in_ })

to

let let_in ?(range = dummy) var ~rhs ~in_ = create ~range (LLTZ.E.Let_in { let_var = var; rhs; in_ })

I think it would be much better to have

let let_in ?(range = dummy) {var; rhs; in_} = create ~range (LLTZ.E.Let_in {let_var=var; rhs; in_})

This way, we always have an optional parameter followed by one mandatory parameter (here, a record, which, conceptually, is a series of labelled parameters). We don't need partial application here anyway. Then

let sender ?(range = dummy) () = create ~range (LLTZ.E.Prim (LLTZ.P.Sender, []))

becomes uniform.

[alanmarkoTrilitech]

My experience with LIGO is that injecting dummies for source location is a temptation that regularly led to error messages without source locations. Like exceptions, they are convenient but often make us look bad. If we use a default parameter with a dummy, all it takes is to forget to pass a location and we have this issue.

I'd rather have a normal version of the instruction set with mandatory locations (ranges) and a specialised version for testing only, with partial evaluation of those to dummies.

Now, to rephrase my issue with the current design (with optionals): we have a fairly large instruction set, so we need a fairly uniform calling convention. For example, you changed

let let_mut_in ?(range = dummy) (var, rhs, in_) = create ~range (LLTZ.E.Let_mut_in { let_var = var; rhs; in_ })

to

let let_in ?(range = dummy) var ~rhs ~in_ = create ~range (LLTZ.E.Let_in { let_var = var; rhs; in_ })

I think it would be much better to have

let let_in ?(range = dummy) {var; rhs; in_} = create ~range (LLTZ.E.Let_in {let_var=var; rhs; in_})

This way, we always have an optional parameter followed by one mandatory parameter (here, a record, which, conceptually, is a series of labelled parameters). We don't need partial application here anyway. Then

let sender ?(range = dummy) () = create ~range (LLTZ.E.Prim (LLTZ.P.Sender, []))

becomes uniform.

We resolved the linker issues with Alistair.

Regarding the arguments, I think it quite complicates the usage of it - e.g. add x y, will become add {lhs=x, rhs=y} which doesn't look very good. Let in is not a very good example as in a further PR I will change it to a let* syntax, something like - let ( let* ) rhs in_ = let xname = Name.create () in let_ xname rhs ~in_:in_ as suggested by Alistair.

I will add @johnyob to this discussion as he was imagining it slightly differently.

[rinderknecht]

{lhs=x, rhs=y} which doesn't look very good.

But it's the same as having labels, like so: ~lhs:x ~rhs:y. That was my point: replace a series of labels by one record.

Let's wait and see what Alistair has to say about this.

[johnyob]

That was my point: replace a series of labels by one record.

I'd be opposed against having a record for each smart constructor. I propose we stick to labelled arguments (using unlabelled arguments only when the context is clear e.g. a single argument or lhs/rhs precedence)

To solve the ?range issue, I propose the following interface (perhaps renaming dsl to ast_builder):

(* ast_builder.mli *)

module type Range = sig 
  val v : Range.t
end
module Dummy_range : Range

module type S = sig
  type 'a with_range

  val let_ : (pattern -> expr -> in_:expr -> expr) with_range

  ...
end

module Default : S with type 'a with_range := range:Range.t -> 'a
module Make (R : Range) : S with type 'a with_range := 'a

include Default
module With_dummy = Make (Dummy_range)

[johnyob]

Shouldn't be merged (added for debugging purposes)

[alanmarkoTrilitech]

fix asserts