IRJ files parser as an Mlang sublibrary #222

mdurero · 2023-07-05T12:31:10Z

Parser for DGFiP test files, AKA "fip files", ".irj", "creneau files", "autotest files"… to load data in a straightforward OCaml record.

Compatible with both actually used file formats right now, primary and corrective computation, and cleaned from unidentified format variations.

Parser is based on original Mlang interpreter parser.

To be used by Mlang interpreter, Mlang OCaml backend, or any external OCaml application.

… used

… for data blocks. It allows for easier integration in Mlang interpreter.

…arser.

…aken from #224. Step in allowing a peaceful merge for #222 and #224.

String conversion now takes place in lexer.

denismerigoux · 2023-11-21T12:58:24Z

@noeensarguet can you push your changes here so that I can review them?

noeensarguet · 2023-11-22T13:29:25Z

@noeensarguet can you push your changes here so that I can review them?

Done
This was mostly reorganizing the directories and including the appropriate dune files to have an independent irj-parser opam package - however I'm not sure that this is very satisfactory as is, we're definitely reaching the limits of flattening the directory structure with the stanza (include_subdirs unqualified)

denismerigoux · 2023-11-22T13:39:21Z

So what you did here is pull all the dependencies of parser_utils.ml into irj_parser but I don't think we can merge this as it destructures the architecture of Mlang completely. The files :

src/mlang/m_frontend/mast.ml
src/mlang/utils/cli.ml
src/mlang/utils/cli.mli
src/mlang/utils/pos.ml
src/mlang/utils/pos.mli

Should remain where they are, hence irj_parser should not depend on them. Hence you should refactor the code of irj_parser/{main.ml, irj_ast.ml, irj_file.ml, irj_parser.mly} to not depend on anything in the list of files above, so that irj_parser can really be a standalone library that does not depend on Mlang. For instance, instead of throwing error messages with Cli.raise_error, the irj_parser library should throw an OCaml exception it defines. It should also embark its mini-AST of the test files.

Keryan-dev · 2023-11-22T13:58:51Z

This is a bit of a tangent.

I agree that depending on MAst is too much, and that errors should be handled in a more library-friendly way (i.e. exceptions or result types). Pos type and functions could be reused but a different minimal version should be enough for the current purpose.

This aside, I would strongly advocate for a much more librarified architecture for Mlang as we may use some of it to build other helpful tools.

denismerigoux · 2023-11-22T14:02:19Z

I agree with you Keryan, we should move to an Mlang architecture where basically each intermediate representation is its own separate library with clear dependence flow between them. That being said I think this PR is a first step toward this : if when this PR is merged irj_parser becomes its own separate library that doesn't depend on the rest of Mlang, we're progressing toward a more modular architecture no?

Keryan-dev · 2023-11-22T14:14:27Z

Yes of course. What I meant is that I would not be shocked to see some adjacent libraries depend on part of the compiler, this one included.

denismerigoux · 2023-11-22T14:40:06Z

Oh yes of course. For instance we'll surely have a utils library that all the intermediate representations, and maybe irj_parser if we want, could depend on.

noeensarguet · 2023-11-23T17:15:08Z

Should be better. Still quite unsatisfied with the (wrapped false) stanza in the irj_parser library, but this was necessary to open Irj_ast in module Test_interpreter. Also opened the definition of type Pos.t to be able to make it compatible with the type t in Irj_ast

denismerigoux

Hi @noeensarguet, thanks very much for the corrections! We're getting there, but there are a few cleaning items that should be done before merging this. This is mostly nitpicking but let's try to make the code as beautiful as possible to set the tone for your beginnings on Mlang :)

denismerigoux · 2023-11-24T09:08:56Z

irj_parser.opam

 synopsis: "Parser for the IRJ tests"
 description:
  "This parser is aimed for the tests used by la DGFiP to test the calculation of the French income tax"
-maintainer: ["Mathieu Durero" " Noé Ensarguet"]


Ha ha why did you remove yourself and Mathieu and put me as the maintainer ;)

Hahaha
I had a separate dune-project for irj_parser in the first commit, until I realised (thanks to @Keryan-dev) that you could declare both packages in the same dune-project file (and so with the same maintainers etc.)

denismerigoux · 2023-11-24T09:09:45Z

src/irj_parser/dune

+(env
+ (static
+  (ocamlopt_flags
+   (-O3 -ccopt -static))))


Are we sure we want -O3 ? That is pretty agressive

This stanza is there in a lot of places and only affect static profile builds (only uses for releases IIRC).

Just checking, are we leaving this as is? I changed the rest following your remarks (thanks for those!) and it's ready to be pushed

Yes you can leave this as is :) Please push!

denismerigoux · 2023-11-24T09:10:02Z

src/irj_parser/main.ml

Why is there this empty file here?

Thought we might want an executable at first and didn't delete it afterwards ^^'

denismerigoux · 2023-11-24T09:10:15Z

src/irj_parser/src/dune

+(env
+ (static
+  (ocamlopt_flags
+   (-O3 -ccopt -static))))


Ditto -O3

denismerigoux · 2023-11-24T09:12:47Z

src/irj_parser/src/irj_ast.ml

+let mk_position sloc =
+  { pos_filename = (fst sloc).Lexing.pos_fname; pos_loc = sloc }
+
+exception


You don't need to duplicate the full type here as you won't use all its components in irj_parser. You can simply declare something like and later do raise TestParsingError(...).

exception TestParsingError of string * pos

denismerigoux · 2023-11-24T09:14:40Z

src/irj_parser/src/irj_file.ml

+  in
+  let f =
+    try Irj_parser.irj_file Irj_lexer.token filebuf with
+    | StructuredError e ->


Here you can catch TestParsingError raised in the parser

denismerigoux · 2023-11-24T09:14:51Z

src/irj_parser/src/irj_file.ml

+    | Irj_parser.Error ->
+        close_in input;
+        raise
+          (StructuredError


And here raise a TestParsingError

denismerigoux · 2023-11-24T09:15:48Z

src/irj_parser/src/irj_parser.mly

+%{ open Irj_ast
+
+  let error (sp, ep) msg =
+    raise (StructuredError ("Parse error : " ^ msg, [ (None, mk_position (sp, ep)) ], None))


Here you can raise a TestParsingError ...

denismerigoux · 2023-11-24T09:21:54Z

src/mlang/test_framework/test_interpreter.ml

-  in
-  close_in input;
-  f
+let convert_pos (sloc : Irj_ast.t) : Pos.t =


Yes perfect!

denismerigoux · 2023-11-24T09:22:58Z

src/mlang/utils/pos.mli

@@ -18,7 +18,9 @@

 (** {2 Source code position} *)

-type t
+type t = { pos_filename : string; pos_loc : Lexing.position * Lexing.position }
+(* opened this type. Not very satisfactory but necessary to enforce the


Instead of opening the type you can just use the make_position function below in test_interpreter.ml/convert_pos :)

Keryan-dev · 2023-11-24T09:25:19Z

I don't think you should need a (wrapped false) to be able to use those module elsewhere. Without it you'd have an additional indirection through the top module Irj_parser : open Irj_parser.Irj_ast didn't do the trick ?

noeensarguet · 2023-11-24T12:44:28Z

@Keryan-dev tried what you said and a couple other things to address the (wrapped false) - maybe I didn't quite wrap my head around this. If I take the stanza out, the only module I can open is Irj_file since it's the top module of the library Irj_parser (and open Irj_parser yields an Unbound module error). Neither open Irj_parser.Irj_ast or open Irj_file.Irj_ast are recognised as bound modules

Besides, if it is possible to add an indirection with open Irj_parser.Irj_ast, I don't quite see what the purpose of the stanza (wrapped _) would be in the first place - so if adding some indirection is an option, I would be down to have some clarifications about that :)

mdurero force-pushed the IRJ-parser-library branch 4 times, most recently from 76d5262 to 13e7160 Compare July 5, 2023 14:02

Report work on parser from ocaml-backend branch

6b00947

mdurero force-pushed the IRJ-parser-library branch from 13e7160 to 6b00947 Compare July 5, 2023 14:32

mdurero added 5 commits July 6, 2023 09:56

Reinstate old empty fields, if by chance some files from 2019 must be…

68ea878

… used

Describe tokens in comments

3d3aec5

Change IRJ file AST structure to use nested records instead of 3-uple…

e633129

… for data blocks. It allows for easier integration in Mlang interpreter.

Make Mlang test interpreter use IRJ parser instead of built-in Test p…

1874466

…arser.

Remove Test parser files

7d7c8ab

mdurero marked this pull request as ready for review November 9, 2023 16:53

mdurero added 2 commits November 10, 2023 14:20

Add flags to Menhir for test file checker compatibility, #224

9938401

With a true "rappel" implementation as records, and some errors, as t…

6cb06e1

…aken from #224. Step in allowing a peaceful merge for #222 and #224.

mdurero marked this pull request as draft November 10, 2023 17:28

mdurero added 8 commits November 10, 2023 19:10

Only moving parts and page layout

fc3ce45

Add NL and EOF where they do not break CI with current lexer

9d59616

Add the last parser checks from #224

c8896bf

Import #224 lexer, adding all NL tokens

cc5f4d6

Explicit calc_error name for errors from 'Calculette'

debeadf

Comments on lexer

d78259f

Use actual integer and float tokens in parser

4114440

String conversion now takes place in lexer.

Comments on design choices

e492f35

denismerigoux mentioned this pull request Nov 21, 2023

WIP: Add a test file checker #224

Closed

3 tasks

denismerigoux marked this pull request as ready for review November 21, 2023 12:57

reorganized to include a irj-parser opam package

900d4c7

irj_parser library (somewhat properly) reorganised

906230c

denismerigoux reviewed Nov 24, 2023

View reviewed changes

Minor corrections

9ca3ac1

denismerigoux merged commit d7fa9bb into master Nov 29, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IRJ files parser as an Mlang sublibrary #222

IRJ files parser as an Mlang sublibrary #222

mdurero commented Jul 5, 2023

denismerigoux commented Nov 21, 2023

noeensarguet commented Nov 22, 2023

denismerigoux commented Nov 22, 2023 •

edited

Loading

Keryan-dev commented Nov 22, 2023

denismerigoux commented Nov 22, 2023

Keryan-dev commented Nov 22, 2023

denismerigoux commented Nov 22, 2023

noeensarguet commented Nov 23, 2023

denismerigoux left a comment

denismerigoux Nov 24, 2023

noeensarguet Nov 24, 2023

denismerigoux Nov 24, 2023

Keryan-dev Nov 24, 2023

noeensarguet Nov 27, 2023

denismerigoux Nov 27, 2023

denismerigoux Nov 24, 2023

noeensarguet Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

denismerigoux Nov 24, 2023

Keryan-dev commented Nov 24, 2023

noeensarguet commented Nov 24, 2023

IRJ files parser as an Mlang sublibrary #222

IRJ files parser as an Mlang sublibrary #222

Conversation

mdurero commented Jul 5, 2023

denismerigoux commented Nov 21, 2023

noeensarguet commented Nov 22, 2023

denismerigoux commented Nov 22, 2023 • edited Loading

Keryan-dev commented Nov 22, 2023

denismerigoux commented Nov 22, 2023

Keryan-dev commented Nov 22, 2023

denismerigoux commented Nov 22, 2023

noeensarguet commented Nov 23, 2023

denismerigoux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Keryan-dev commented Nov 24, 2023

noeensarguet commented Nov 24, 2023

denismerigoux commented Nov 22, 2023 •

edited

Loading