Merge pull request #779 from diml/fix-sexp-record-bug

Fix sexp record bug

CHANGES.md

@@ -43,6 +43,9 @@ next
 - Add an `ignored_subdirs` stanza to replace `jbuild-ignore` files
   (#767, @diml)
 
+- Fix a bug where Dune ignored previous occurences of duplicated
+  fields (#779, @diml)
+
 1.0+beta20 (10/04/2018)
 -----------------------
 

src/stdune/sexp.ml

@@ -273,6 +273,8 @@ module Of_sexp = struct
           | List (_, name_sexp :: values) -> begin
               match name_sexp with
               | Atom (_, A name) ->
+                if Name_map.mem acc name then
+                  of_sexp_errorf sexp "Field %S is present too many times" name;
                 Name_map.add acc name { values; entry = sexp }
               | List _ | Quoted_string _ ->
                 of_sexp_error name_sexp "Atom expected"

test/unit-tests/dune

@@ -69,3 +69,13 @@
            (progn
             (run ${exe:expect_test.exe} ${<})
             (diff? ${<} ${<}.corrected))))))
+
+(alias
+ ((name runtest)
+  (deps (sexp.mlt
+         (glob_files ${SCOPE_ROOT}/src/.dune.objs/*.cmi)
+         (glob_files ${SCOPE_ROOT}/src/stdune/.stdune.objs/*.cmi)))
+  (action (chdir ${SCOPE_ROOT}
+           (progn
+            (run ${exe:expect_test.exe} ${<})
+            (diff? ${<} ${<}.corrected))))))

test/unit-tests/sexp.mlt

@@ -0,0 +1,33 @@
+(* -*- tuareg -*- *)
+open Stdune;;
+open Sexp.Of_sexp;;
+
+let pp_sexp_ast ppf sexp =
+  Sexp.pp ppf (Sexp.Ast.remove_locs sexp)
+;;
+#install_printer pp_sexp_ast;;
+[%%expect{|
+val pp_sexp_ast : Format.formatter -> Stdune.Sexp.Ast.t -> unit = <fun>
+|}]
+
+Printexc.record_backtrace false;;
+[%%expect{|
+- : unit = ()
+|}]
+
+let sexp = Sexp.parse_string ~fname:"" ~mode:Single {|
+((foo 1)
+ (foo 2))
+|}
+[%%expect{|
+val sexp : Usexp.Ast.t = ((foo 1) (foo 2))
+|}]
+
+let of_sexp = record (field "foo" int)
+let x = of_sexp sexp
+[%%expect{|
+val of_sexp : int Stdune.Sexp.Of_sexp.t = <fun>
+Exception:
+Stdune__Sexp.Of_sexp.Of_sexp (<abstr>,
+ "Field \"foo\" is present too many times", None).
+|}]