OCaml backend #148
OCaml backend #148
Conversation
examples/ocaml/test_harness.ml
Outdated
| match line_list with | ||
| | [] -> List.rev (List.map List.rev line_list_list) | ||
| | line :: tail -> | ||
| if String.starts_with ~prefix:"#" line then |
There was a problem hiding this comment.
String.starts_with comes with ocaml 4.13, Mlang aims to be compatible with 4.11.2 (cf. the .opam file).
There was a problem hiding this comment.
Should be fixed by using the parser from the test_framework in the main mlang codebase, instead of doing the same job badly and buggyly.
There was a problem hiding this comment.
The backend is now usable with compiler 4.11.2.
I have copied selected parts of Mlang internal parser and test parser in order to obtain a independent parser for DGFiP test format. It is incomplete (as it was in Mlang test), but I think it would be a good idea to extend it.
|
Native compilation of the generated code file causes a stack overflow, due to multiple huge dynamic data structures for entries and outputs. As a first approach, we could modify generated code to build the corresponding static arrays as the first step, then apply relevant transformation to compute the dynamic parts and convert to the target data structure. Update : building purely static array of the variable positions then transforming it to values array or to a position map seems to be enough to solve the overflow. |
| else floor (x.value +. 0.50005)); | ||
| } | ||
|
|
||
| let m_null = m_not |
examples/ocaml/mvalue.ml
Outdated
| if bound >= 1 then | ||
| multimax variable_array (position + 1) (position + bound) | ||
| (get_position_value position) | ||
| else get_position_value position |
There was a problem hiding this comment.
This if would not be needed if the exit condition above was more robust.
Hint: you can write the whole thing more concisely with Array.sub and Array.fold.
I don't doubt the current implementation is correct, mind.
examples/ocaml/mvalue.ml
Outdated
| m_max reference (get_position_value current_index) | ||
| in | ||
| if current_index = max_index then new_max | ||
| else multimax variable_array (current_index + 1) max_index new_max |
There was a problem hiding this comment.
This is fine until you call multimax with a start index greater than the max index.
There was a problem hiding this comment.
Technically multimax only initial call was after the bound ≥ 1 check, so start index being postion+1 and max index position+bound, having start index greater than max index shouldn't have been possible. But I'm trying an Array.fold_left based implementation.
There was a problem hiding this comment.
That's my point above: you shouldn't need the >= 1 check with a stronger exit condition. But all this won't matter with a fold anyway.
There was a problem hiding this comment.
I agree. Well I don't know how, but I manage to break it with the fold…
|
I'd like to have #184 merged to have a more clean diff on this PR for reviewing. @denismerigoux ? As an early thought: parts of it rely a lot on makefiles whereas we could use dune for a less tedious build system. But it works as it is, so we can just keep that in mind for later patches. |
|
Also could you add this backend to the CI ? |
|
Making the test parser an independent library and fleshing it out to support all our test files is precisely my plan, if I'm not asked to prioritise something else. As for adding the backend to the CI, I have this in my Git stash since I don't know when, applying it when I need it. At that time I was squeamish about modifying Mlang code base so I never committed the changes on the main Makefile. Just have to check if I use custom test files because I had to tailored some of them to test M errors when I added them. |
|
#184 merged |
| let bindings_list = Mir.IndexMap.bindings es in | ||
| Format.pp_print_list ~pp_sep:pp_statement_separator | ||
| (fun fmt (i, v) -> | ||
| generate_one_var (get_var_pos variable |> ( + ) i) fmt v) |
There was a problem hiding this comment.
Well, it certainly is one way to write this 😀
| let cond_name = | ||
| Format.asprintf "cond_%s_%d_%d_%d_%d" fname (Pos.get_start_line pos) | ||
| (Pos.get_start_column pos) (Pos.get_end_line pos) | ||
| (Pos.get_end_column pos) |
There was a problem hiding this comment.
Is this for debugging purposes ? Seems a bit overkill for a shadowable name.
There was a problem hiding this comment.
I would say: "probably close to how it was done in other backends" and the only use I picture for it in the foreseeable future is helping generated code readability.
| {undefined = false ; value = entry_var.value} in@,\ | ||
| List.iter init_tgv_var entry_list" pp_print_position_map input_vars | ||
| (* Prévoir les cas : variable manquante, variable en trop dans entry_list, | ||
| variable définie n fois*) |
There was a problem hiding this comment.
Good catch. No it isn't. Seems like all tests are kindly providing well built entry lists. Not so obvious a check to add, though.
There was a problem hiding this comment.
Don't bother, just put a comment stating we don't check those cases.
…ingle FIP file or all files from a test directory.
…n the output variable list, generally because it's an input variable, so it is pointless to mark it as output.
… calculate_tax function. Test command "raw" now writes also the list of triggered M errors.
…pec file, .mpp file and main MPP function, to run test suite with bytecode and native code.
…ed like any other M rule.
…, with better display of M Errors.
…atenation on this huge list.
…es in other backends
…variable position instead of computing values or map line by line
…vant code from Mlang main and interpreter parsers
…y and enable native code compilation
…ing M table variables
…s function in backends, 2022-11-22)
…primary and corrective test files.
…rser need to be done before using it.
No description provided.