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
-dsource output of ppx-deriving show has errant "in" #7111
Comments
Comment author: furuse This happens when Pexp_let has an empty list for its binding. You should report it at ppx_deriving issues too. In pprintast.ml, bindings method does nothing when the binding list is empty. I guess it should raise an exception instead. |
Comment author: @alainfrisch Is anyone opposed to having the type-checker reject empty let bindings? |
Comment author: @diml I'm for rejecting empty let bindings. There are a few other inconsistencies between the typer and pprintast. For instance empty P(str|sig)_type are ignored by the typer but cause an [assert false] in pprintast. To improve things we can share the ill_formed_ast errors in the Syntaxerr module: val empty_tuple : Location.t -> 'a This is strictly better than getting [assert false] or incorrect output with [-dsource]. But I think it would be even better to check all the AST invariants once and for all after applying the ppx rewriters. To ensure that we are testing the same invariants between the typer and pprintast. I'll look into this. |
Comment author: @alainfrisch
Indeed. In addition to ensuring coherence between various "clients" of the Parsetree, it will formally document the invariants which are not expressed in the Parsetree definition. I'll still prefer to reject empty let bindings in 4.03, even if this would be subsumed by your more ambitious idea later on. Or do you think you can work on it very soon? Of course, a first version does not need to enforce absolutely all invariants. |
Comment author: @diml
I have a first working version that subsumes all the uses of [Syntaxerr.ill_formed_ast]. I'll create a PR this afternoon |
Comment author: @damiendoligez Empty let-bindings are rejected since #392 was merged. |
Original bug ID: 7111
Reporter: leonidr
Status: closed (set by @xavierleroy on 2017-09-24T15:31:47Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.3
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: ~DO NOT USE (was: OCaml general)
Related to: #7002
Monitored by: @hcarty
Bug description
See steps below. I've tried it with various type declarations; all of the ones on the ppx_deriving documentation.
Steps to reproduce
$ cat test.ml
type a = Foo [@@deriving show]
$ PPD=
ocamlfind query ppx_deriving
$ ocamlc -c -I $PPD -ppx "$PPD/ppx_deriving $PPD/ppx_deriving_show.cma" -dsource test.ml 2> test_part2.ml
$ echo $?
0
$ cat test_part2.ml
type a =
| Foo[@@deriving show]
let rec pp_a : Format.formatter -> a -> Ppx_deriving_runtime.unit=
in
((let open! Ppx_deriving_runtime in
fun fmt -> function | Foo -> Format.pp_print_string fmt "Test.Foo")
[@ocaml.warning "-A"])
and show_a : a -> Ppx_deriving_runtime.string=
fun x -> Format.asprintf "%a" pp_a x
The text was updated successfully, but these errors were encountered: