Skip to content

About label.ml naming, comments, improvements

There are several problems with the module label.ml. These could be reported in several separated issues but are somehow intertwined so i prefer keeping them together.

  1. The name "label" is poorly chosen, it should be named "conftrolflow" (or simply "flow"), as it is not a representation of the labels of a program but rather a structure used to guide the evaluation through the flow of control.
  2. This module is central to the evaluator, and complicated, so it deserves to be thoroughly commented. A main comment at the start of the module explaining its main goal could be appreciated.
  3. label end Should be treated differently than other labels. See next point.
  4. It is unclear what is the main type of the module. What to we compute? AFAIU, we must build a representation of the program, where we store litterals and blocks. I think we should add more structure and make appear also procedures/routines (aka functions) which will be needed anyway at some point, to perform static analysis that are interprocedural A type somewhat like the following is what i have in mind:
type t = {
  litterals: string SMap.t;
  routines: block SMap.t;
}

and block = { ... } (* jump_dest? *)

Having a type named t (along with its printer named print) makes it already more explicit what we're doing. Preload should return a value of type t ideally.

And then you can build a value of type t by doing more or less:

let build (prog : stmt list) : t =
  let rec loop acc ((curname, curbody) as cur) = function
    | [] when curbody = [] -> acc
    | [] ->
        Format.eprintf
          "The routine %s should end with 'label end'. Terminating it.\n"
          curname ;
        {acc with routines= SMap.add curname (List.rev curbody) acc.routines}
    | Litteral (n, str) :: t ->
        loop {acc with litterals= SMap.add n str acc.litterals} cur t
    | (Label "end" as l) :: t ->
        loop
          { acc with
            routines= SMap.add curname (preload (List.rev (l :: curbody))) acc.routines
          }
          ("", []) t
    | Label n :: t
      when curbody = [] (* this means the label is at top level *) ->
        loop acc (n, []) t
    | stmt :: t -> loop acc (curname, stmt :: curbody) t
  in
  loop {litterals= SMap.empty; routines= SMap.empty} ("", []) prog

That way, the labels end are not stored, as they are not needed other than for delimiting the different routines. Disclaimer : i have not tested this code, likely not to compile.

These are only suggestions, feel free to disagree with some (or all) of the points above.