Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007111OCaml~DO NOT USE (was: OCaml general)public2016-01-01 22:202017-09-24 17:31
Reporterleonidr 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version4.02.3 
Target Version4.03.0+dev / +beta1Fixed in Version4.03.0+dev / +beta1 
Summary0007111: -dsource output of ppx-deriving show has errant "in"
DescriptionSee 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
TagsNo tags attached.
Attached Files

- Relationships
related to 0007002closed Empty let-bindings accepted by Ast_helper / pretty printer 

-  Notes
(0015213)
furuse (reporter)
2016-01-04 03:33

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.
(0015214)
frisch (developer)
2016-01-04 09:56

Is anyone opposed to having the type-checker reject empty let bindings?
(0015215)
dim (developer)
2016-01-04 12:33

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
  val empty_let : 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.
(0015217)
frisch (developer)
2016-01-04 13:36

> But I think it would be even better to check all the AST invariants once and for all after applying the ppx rewriters.

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.
(0015219)
dim (developer)
2016-01-04 13:55

> 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.

I have a first working version that subsumes all the uses of [Syntaxerr.ill_formed_ast]. I'll create a PR this afternoon
(0015221)
dim (developer)
2016-01-04 15:07

https://github.com/ocaml/ocaml/pull/392 [^]
(0015307)
doligez (administrator)
2016-02-03 14:52

Empty let-bindings are rejected since https://github.com/ocaml/ocaml/pull/392 [^] was merged.

- Issue History
Date Modified Username Field Change
2016-01-01 22:20 leonidr New Issue
2016-01-04 03:33 furuse Note Added: 0015213
2016-01-04 09:56 frisch Note Added: 0015214
2016-01-04 12:33 dim Note Added: 0015215
2016-01-04 13:36 frisch Note Added: 0015217
2016-01-04 13:55 dim Note Added: 0015219
2016-01-04 15:07 dim Note Added: 0015221
2016-02-03 14:49 doligez Relationship added related to 0007002
2016-02-03 14:52 doligez Note Added: 0015307
2016-02-03 14:52 doligez Status new => resolved
2016-02-03 14:52 doligez Resolution open => fixed
2016-02-03 14:52 doligez Fixed in Version => 4.03.0+dev / +beta1
2016-02-03 14:52 doligez Target Version => 4.03.0+dev / +beta1
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-03-03 17:55 doligez Category -OCaml general => -(deprecated) general
2017-03-03 18:01 doligez Category -(deprecated) general => ~deprecated (was: OCaml general)
2017-03-06 17:04 doligez Category ~deprecated (was: OCaml general) => ~DO NOT USE (was: OCaml general)
2017-09-24 17:31 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker