Ast_pattern: augment API with ebool, pbool helper, and a new map.

[panglesd]

estring, echar, efloat, ... helpers are created using functions generated from the constant type.

However, this type does not contain boolean constants, such as true and false. Indeed, such constants are defined as constructors (without argument, nor uppercase), as seen using utop -dparsetree:

utop # true ;;
Ptop_def
  [
    structure_item (//toplevel//[1,0+0]..[1,0+4])
      Pstr_eval
      expression (//toplevel//[1,0+0]..[1,0+4])
        Pexp_construct "true" (//toplevel//[1,0+0]..[1,0+4])
        None
  ]

As a consequence, ebool was not added to the Ast_pattern API, even though it is useful (as illustrated here)

It would be useful to have such values in the API:

val ebool : (bool, 'a, 'b) t -> (expression, 'a, 'b) t
val pbool : (bool, 'a, 'b) t -> (pattern, 'a, 'b) t

While looking at this, I noticed that we are also missing two maps:

val map_value : f:('a -> 'b) -> ('b, 'c, 'd) t -> ('a, 'c, 'd) t
val map_value' : f:(location -> 'a -> 'b) -> ('b, 'c, 'd) t -> ('a, 'c, 'd) t

with which it is simple to generate ebool and pbool, and that should be added to the API as well!

[Burnleydev1]

Hello @panglesd, please I would like to work on this issue.

[panglesd]

Hello @Burnleydev1!

Great, feel free to ask me any question you would have: Ast_pattern can be confusing!

[Burnleydev1]

Thanks @panglesd, my first question may be offtopic, I wish to ask if there is a way for me to locally build the API website.

[panglesd]

Sure! You can run dune build @doc. This will generate the doc, the entry point to it is _build/default/_doc/index.html.

[panglesd]

Some potentially useful information: some part of the code of Ast_pattern is generated in the Ast_pattern_generated module.
If you want to see what it looks like, you can find it at _build/default/src/ast_pattern_generated.ml after having run dune build.

[Burnleydev1]

Thanks, @panglesd, if I am not mistaken, it means that I need to add those values in such a way that it shows on the api in the manner you described in the comment above?

[Burnleydev1]

Hello @panglesd, please correct me if I am wrong, I think I understand now, what we want to do is to add a constant like pconst_bool so that it can be used to achieve something like this val ebool : (bool, 'a, 'b) t -> (expression, 'a, 'b) t
I noticed that the only bool was pdir_bool, but it is not compatible with pexp_constant.

[Burnleydev1]

I may be totally wrong, but the reason I stated the above observation is because when i use pdir_bool, i get this error

(bool, 'a, 'b) t -> (directive_argument, 'a, 'b) t

This expression has type (directive_argument, 'a, 'b) t
but an expression was expected of type (constant, 'c, 'd) t
Type directive_argument is not compatible with type constant

[panglesd]

It is important to understand well the AST types in order to solve this issue!

The AST type can be found here: https://ocaml.org/p/ppxlib/latest/doc/Astlib/Ast_500/Parsetree/index.html
Most types and constructors are explained with a small example to what they correspond to. For instance, the constant type has several variant: Pconst_integer, Pconst_char, ... which correspond to different type of constants: for instance 14 would be represented as Pconst_integer(14, None).

If you look at directive_argument, you can see that they correspond to arguments to toplevel phrases. (toplevel phrases are used to control the toplevel behavior, see it in the manual).
This has nothing to do with what we are interested in (expression), so let's ignore it now!

We are interested in how to turn a bool into an expression.
If you look at the expression type, there are many ways to build an expression. For constants, you use Pexp_constant, but as you can see in the constant type, this is not what is used to define the true and false constant booleans.

If you want to know the AST that corresponds to a specific expression, you can run utop with the -dparsetree option. This will make the toplevel to pretty print the AST value corresponding to what you sent to the toplevel. You can also use https://astexplorer.net/ for this.

If you use any of the tool to know the AST corresponding to true, you will see that true is represented using Pexp_construct, the variant to represent constructors!

Before adding ebool and pbool to Ast_pattern, I think it would be a good warm-up to add ebool and pbool to Ast_builder. Ast_builder and Ast_pattern are quite similar, but the former one is easier to manipulate, and less confusing. You can find some documentation on Ast_builder here!

So, the function to add to Ast_builder would have type:

(* in Ast_builder.Default *)
val ebool : ~loc:location -> bool -> expression
val pbool : ~loc:location -> bool -> pattern

(* in Ast_builder.Make *)
val ebool : bool -> expression
val pbool : bool -> pattern

They would respectively build an expression or a pattern representing a boolean, from such a boolean. To build such values, you can use other values of Ast_builder such as pexp_construct, or directly the AST type. (You won't need the map I was referring to in Ast_pattern, forget about this one for now!)

When I say "add to the API", I mean:

• Add an implementation of the functions in the .ml file,
• Add the signature of the function to the .mli file, so that they are accessible by other modules

and that's it! They will automatically appear in the documentation.

I hope it is clearer what you have to do, and how!

[Burnleydev1]

@panglesd, Thanks a lot for the pointers, I will get on it right away.

[Burnleydev1]

Hello @panglesd, I noticed that ebool and pbool already exist on ast_builder.Default and Ast_builder.Make.

[panglesd]

Good point, sorry I missed that!

In this case, you can try the Ast_pattern version.

[Burnleydev1]

Okay @panglesd, I will get on that right away.

[Burnleydev1]

Hello @panglesd, Ast_pattern really is tricky
I wish to ask if

  if b then
    pexp_construct (Longident.Lident "true")

  else
    pexp_construct (Longident.Lident "false")"

or

let ebool b =
  if b then
    pexp_construct (pconst_string "true" b)

  else
    pexp_construct ( pconst_string "false")

is a good start to solving this issue
I am also confused about if the constructor to use here because the result of utop -dparsetree shows pexp_construct "true" or "false" and None, but the result of other helpers like estring has Pexp_constant and PConst_string and PConst_string is used but with the bool it is not stated there.

[panglesd]

I think the first thing is to understand well Ast_pattern.

For this, you need to read (again, with fresh mind!) the manual on it: here and understand how to use it.
But, the manual does not say how all of this is implemented. So, you should also look at the implementation as well!
There are two things that you should spend some time on, I think:

• First, understand well the concrete type Ast_pattern.t. You can find it here, but I'll copy it here:

type ('matched_value, 'k, 'k_result) t =
  | T of (context -> Location.t -> 'matched_value -> 'k -> 'k_result)

So, an Ast_pattern is just a function (enclosed in a constructor) which takes several things: a context, a location, the value to match on, a continuation (see the manual on this), and use all that to produce a result.

• Now that you know the Ast_pattern.t type, you can test this understanding by looking at functions involving this type: parse_res, __, map, ..., as well as all function that you can find in _build/default/src/ast_pattern_generated.ml (the file exists only after having run dune build, as those functions are generated from the AST type at compile time).

At this point, Ast_pattern should start to feel less mysterious. You should be able to start coding. I think you have two choices:

• Either you take inspiration from pexp_construct in ast_pattern_generated.ml, and e.g. some in ast_pattern.ml, to directly generate ebool of the right type,
• Or, and I think it is better (but maybe slightly less easy to write?) you do the following steps:
  • Write a function map_value : f:('a -> 'b) -> ('b, 'c, 'd) Ast_pattern.t -> ('a, 'c, 'd) Ast_pattern.t, which is very similar to map, but maps the value instead of the continuation.
  • Then, you use this function to turn an (bool, 'c, 'd) Ast_pattern.t into a (string, c, d) Ast_pattern.t.
  • Finally, you combine pexp_construct, lident and the value previously create to define ebool.

I am also confused about if the constructor to use here because the result of utop -dparsetree shows pexp_construct "true" or "false" and None, but the result of other helpers like estring has Pexp_constant and PConst_string and PConst_string is used but with the bool it is not stated there.

This is because of how the OCaml AST has been designed: some values (strings, integer, float, ...) were encoded as constants, but others (true and false, notably) are encoded as special constructors. Pexp_construct takes an expression option as second argument (representing the arguments to the constructor), thus the None in the "true" case. Pexp_constant is different from Pexp_construct.

That's why you have to write ebool differently from estring. Look at the ebool and estring versions in Ast_builder: they are written differently.

[panglesd]

I wish to ask if

  if b then
    pexp_construct (Longident.Lident "true")

  else
    pexp_construct (Longident.Lident "false")"

or

let ebool b =
  if b then
    pexp_construct (pconst_string "true" b)

  else
    pexp_construct ( pconst_string "false")

is a good start to solving this issue

Definitely a good start! However, in your two attempts, b is a boolean. In the ebool we want, b has to be of type (bool, 'a, 'b) Ast_pattern.t. Otherwise, we cannot write things such as ebool __ to extract a boolean from an expression.

[Burnleydev1]

Thanks a lot for the pointers @panglesd, I will get on it right away.

[Burnleydev1]

Hi @panglesd, I decided to use your second method to go about the solution,applying the (match, continuation, result) concept, does this mean that the signature of map_value will be something like this: val map_value : ('a, 'b, 'c)t -> f:('d -> 'a) -> ('d, 'b, 'c)t?

[Burnleydev1]

I will create a pull request to track the changes

[Burnleydev1]

Hello @panglesd, I am trying to implement Lident but I am facing difficulties, are there some examples of how it is used with pexp_construct, I would love to see as the only implementation I saw in `Ast_pattern is not really clear, I succeeded to carry out the first 2 steps(I think).

[panglesd]

I'll answer in the PR directly