Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parsexp: constraint ocaml to < 4.13 #18966

Merged
merged 1 commit into from
Jun 30, 2021
Merged

parsexp: constraint ocaml to < 4.13 #18966

merged 1 commit into from
Jun 30, 2021

Conversation

rjbou
Copy link
Contributor

@rjbou rjbou commented Jun 28, 2021

Detected in #18919

#=== ERROR while compiling parsexp.v0.14.0 ====================================#
# context              2.1.0~rc | linux/x86_64 | ocaml-variants.4.13.0+trunk | file:///src
# path                 ~/.opam/4.13/.opam-switch/build/parsexp.v0.14.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p parsexp -j 47
# exit-code            1
# env-file             ~/.opam/log/parsexp-20-72a07e.env
# output-file          ~/.opam/log/parsexp-20-72a07e.out
### output ###
#       ocamlc src/.parsexp.objs/byte/parsexp__Parser_automaton_internal.{cmo,cmt} (exit 2)
# (cd _build/default && /home/opam/.opam/4.13/bin/ocamlc.opt -w -40 -g -bin-annot -I src/.parsexp.objs/byte -I /home/opam/.opam/4.13/lib/base/caml -I /home/opam/.opam/4.13/lib/sexplib0 -intf-suffix .ml -no-alias-deps -open Parsexp__ -o src/.parsexp.objs/byte/parsexp__Parser_automaton_internal.cmo -c -impl src/parser_automaton_internal.ml)
# File "src/parser_automaton_internal.ml", lines 440-454, characters 6-66:
# 440 | ......match state.kind with
# 441 |       | Positions ->
# 442 |         (* Note we store end positions as inclusive in [Positions.t], so we use [delta:0],
# 443 |            while in the [Cst] case we save directly the final ranges, so we use
# 444 |            [delta:1]. *)
# ...
# 451 |           add_pos state ~delta:0;
# 452 |           make_list [] stack)
# 453 |         else stack
# 454 |       | Cst -> make_list_cst (current_pos state ~delta:1) [] stack
# Warning 18 [not-principal]: 
#   The return type of this pattern-matching is ambiguous.
#   Please add a type annotation, as the choice of `s' is not principal.
# File "src/parser_automaton_internal.ml", line 485, characters 44-66:
# 485 |     | Sexp -> if is_not_ignoring state then Sexp (Atom str, stack) else stack
#                                                   ^^^^^^^^^^^^^^^^^^^^^^
# Error: The constructor Sexp expects 0 argument(s),
#        but is applied here to 1 argument(s)
#     ocamlopt src/.parsexp.objs/native/parsexp__Parser_automaton_internal.{cmx,o} (exit 2)
# (cd _build/default && /home/opam/.opam/4.13/bin/ocamlopt.opt -w -40 -g -I src/.parsexp.objs/byte -I src/.parsexp.objs/native -I /home/opam/.opam/4.13/lib/base/caml -I /home/opam/.opam/4.13/lib/sexplib0 -intf-suffix .ml -no-alias-deps -open Parsexp__ -o src/.parsexp.objs/native/parsexp__Parser_automaton_internal.cmx -c -impl src/parser_automaton_internal.ml)

@kit-ty-kate
Copy link
Member

kit-ty-kate commented Jun 28, 2021

This is a bug in OCaml 4.13 not in parsexp, see ocaml/ocaml#10364

@mseri mseri closed this Jun 28, 2021
@kit-ty-kate
Copy link
Member

It is now unsure if this is going to get fixed in the compiler or if it is part of an expected behaviour. parsexp.v0.14.1 is fixing that on its own regardless: #18977

Reopening.

@kit-ty-kate kit-ty-kate reopened this Jun 30, 2021
@avsm
Copy link
Member

avsm commented Jun 30, 2021

Lets get our metadata consistent with the current state of affairs. This is fine to merge now, and we can revert the constraint if 4.13 fixes the bug by release.

@avsm avsm merged commit 92ea452 into ocaml:master Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants