Add end position

[rjbou]

Changes types of OpamParserTypes, to foo_kind the main type value and foo the type with a position

type file_name = string
type pos =
  { filename: file_name;
    start: int * int; (* line, column *)
    stop: int * int; (* line, column *)
  }
type 'a with_pos =
  { pelem : 'a;
    pos : pos
  }
type value_kind =
  | Int of int
  [...]
and value = value_kind with_pos

[dra27]

Sorry to only just be looking at this, but isn’t a problem to break anything which uses the library? It should be possible to have the parser produced the more-detailed tree as a new entry point and have a function to transform it using the name of the old entry point?

[rjbou]

On the lib dependencies, I already ported most of them (opam, opam-ed, conex, kept travis-opam), and a test suite is needed to ensure that nothing is broken (I discovered bugs with conex test suite 🙄 ).
We can't keep the same datatype, as position was given only on their starting char, and not for everything (value itself has no position, only the beginning one of its first component, etc.). But it is possible to add a compatibility module, to give some translation functions.

[rjbou]

Updated. Some duplicated code to avoid double transformation.
Added also deprecation notice, maybe better to add it in another PR and keep it for another release, to let people the time to update before they have the deprecation error.

[rjbou]

rebased, test ported, merging once CI finish

[dra27]

I'm sorry for taking so long to look at this again! The review looks horrific, but in fact it's mainly suggestions to eliminate the shift/reduce warnings in the parser, structure the types slightly differently in OpamParserTypes (I think it is an error that you can't have OpamParserTypes.FullPos.Eq, for example) and shift some doc comments around.

[rjbou]

Thanks for the review!

[dra27]

The build failure on 4.02 is a bit tedious, but can be fixed with:

diff --git a/src/dune b/src/dune
index 1c55e14..508e8d1 100644
--- a/src/dune
+++ b/src/dune
@@ -3,7 +3,16 @@
   (public_name opam-file-format)
   (synopsis "Parser and printer for the opam file syntax")
   (wrapped false)
+  (flags :standard (:include flags.sexp))
   (modules_without_implementation opamParserTypes))

+(rule
+  (with-stdout-to flags.ml
+    (echo "print_string (if String.sub Sys.ocaml_version 0 5 = \"4.02.\" then \"(-w -50)\" else \"()\")")))
+
+(rule
+  (with-stdout-to flags.sexp
+    (run ocaml %{dep:flags.ml})))
+

(unfortunately attributes are not processed during parsing)

[dra27]

Typo in tests/fullpos.ml is blocking the 4.02/4.03 tests passing.

Just need to eliminate OpamParserTypes.Common and rename the rel_op, etc. types and I think we're there

[AltGr]

Requires OCaml >= 4.02.2

[dra27]

We're already in 4.02 territory with the .mli files because we're using attributes for the deprecations. Do we really need to worry about < 4.02.3?

[rjbou]

In the test, it is 4.02.3 the oldest one

[dra27]

FWIW, this restores < 4.02.2 support 🙂

diff --git a/src/Makefile b/src/Makefile
index e1eb5f6..d3c8c17 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -3,17 +3,31 @@ all: opam-file-format.cmxa opam-file-format.cma
 %.ml: %.mll
        ocamllex $<

+OCAML_VERSION:=$(shell ocamlc -vnum | tr -d '\r' | sed -e 's/+.*//' -e 's/\.//g')
+ifeq ($(OCAML_VERSION),)
+OCAML_VERSION:=0
+endif
+
+ifeq ($(shell test $(OCAML_VERSION) -ge 4022 || echo no-attributes),)
+PP=
+else
+PP=-pp 'sed -e s/\\[@@\\?ocaml[^]]*\\]//'
+endif
+
+show:
+       echo $(OCAML_VERSION)
+
 %.ml %.mli: %.mly
        ocamlyacc $<

 %.cmi: %.mli
-       ocamlc -c $<
+       ocamlc -c $(PP) $<

 %.cmo: %.ml %.cmi
-       ocamlc -c $<
+       ocamlc -c $(PP) $<

 %.cmx: %.ml %.cmi
-       ocamlopt -c $<
+       ocamlopt -c $(PP) $<

 MODULES = $(sort $(basename $(wildcard *.ml *.mll *.mly)))

@@ -41,6 +55,6 @@ clean:
        rm -f *.cm* *.o *.a $(GENERATED) .depend .merlin

 .depend: *.ml *.mli $(GENERATED)
-       ocamldep *.mli *.ml > .depend
+       ocamldep $(PP) *.mli *.ml > .depend

 -include .depend
diff --git a/src/opamParserTypes.mli b/src/opamParserTypes.mli
index 601201b..f57f3f3 100644
--- a/src/opamParserTypes.mli
+++ b/src/opamParserTypes.mli
@@ -45,7 +45,7 @@ type env_update_op = Eq       (** [=] *)
 module FullPos : sig

   (** Source file positions *)
-  type nonrec file_name = file_name
+  type file_name = string

   (** Full position *)
   type pos = {

[rjbou]

all green \o/