Pattern type constraint in let expression parsed inconsistently #7758

vicuna · 2018-03-25T17:18:35Z

Original bug ID: 7758
Reporter: antron
Status: new
Resolution: open
Priority: normal
Severity: minor
Version: 4.06.1
Category: lexing and parsing
Monitored by: @nojb @Drup @ygrek

Bug description

The expression

(1) let foo : int = 42 in ...

seems to get parsed as

 `let foo : int = 42 : int in ...`

while the expression

(2) let (foo : int) = 42 in ...

which differs only in having parentheses, seems to get parsed as

  `let foo : int = 42 in ...`

I would have expected the second parse for both cases. The first parse can be problematic when using PPX, because the variable and expression may need to have different types. In particular, in

  `let%lwt foo : int = bar in ...`

foo should have type int, and bar should have type int Lwt.t. See ocsigen/lwt#564.

Steps to reproduce

source.ml:

let () =
  let foo : int = 42 in
  let (foo : int) = 42 in
  ignore foo

ocamlopt -dparsetree -c source.ml
Result is:

[
  structure_item (source.ml[1,0+0]..[4,59+12])
    Pstr_value Nonrec
    [
      <def>
        pattern (source.ml[1,0+4]..[1,0+6])
          Ppat_construct "()" (source.ml[1,0+4]..[1,0+6])
          None
        expression (source.ml[2,9+2]..[4,59+12])
          Pexp_let Nonrec
          [
            <def>
              pattern (source.ml[2,9+6]..[2,9+20]) ghost
                Ppat_constraint
                pattern (source.ml[2,9+6]..[2,9+9])
                  Ppat_var "foo" (source.ml[2,9+6]..[2,9+9])
                core_type (source.ml[2,9+6]..[2,9+20]) ghost
                  Ptyp_poly
                  core_type (source.ml[2,9+12]..[2,9+15])
                    Ptyp_constr "int" (source.ml[2,9+12]..[2,9+15])
                    []
              expression (source.ml[2,9+6]..[2,9+20]) ghost
                Pexp_constraint   <----------------------------- !!!!
                expression (source.ml[2,9+18]..[2,9+20])
                  Pexp_constant PConst_int (42,None)
                core_type (source.ml[2,9+12]..[2,9+15])
                  Ptyp_constr "int" (source.ml[2,9+12]..[2,9+15])
                  []
          ]
          expression (source.ml[3,33+2]..[4,59+12])
            Pexp_let Nonrec
            [
              <def>
                pattern (source.ml[3,33+6]..[3,33+17])
                  Ppat_constraint
                  pattern (source.ml[3,33+7]..[3,33+10])
                    Ppat_var "foo" (source.ml[3,33+7]..[3,33+10])
                  core_type (source.ml[3,33+13]..[3,33+16])
                    Ptyp_constr "int" (source.ml[3,33+13]..[3,33+16])
                    []
                expression (source.ml[3,33+20]..[3,33+22])
                  Pexp_constant PConst_int (42,None)
            ]
            expression (source.ml[4,59+2]..[4,59+12])
              Pexp_apply
              expression (source.ml[4,59+2]..[4,59+8])
                Pexp_ident "ignore" (source.ml[4,59+2]..[4,59+8])
              [
                <arg>
                Nolabel
                  expression (source.ml[4,59+9]..[4,59+12])
                    Pexp_ident "foo" (source.ml[4,59+9]..[4,59+12])
              ]
    ]
]

Additional information

Further complicating this in practice is that in 4.05.0, (1) is parsed as

  `let foo = 42 : int in`

i.e. the constraint is removed from the pattern and appears on the expression only.

The text was updated successfully, but these errors were encountered:

vicuna · 2018-04-11T11:37:00Z

Comment author: @Drup

It would be nice to sort this out before the next release, as it messes up with ppx in a way that is difficult to take into account, even with ocaml-migrate-parsetree.

If someone has a concrete proposition on how to make this cleaner, I'm ready to implement it.

vicuna · 2018-10-15T11:46:33Z

Comment author: @alainfrisch

See https://blog.janestreet.com/plans-for-ocaml-408/, "https://blog.janestreet.com/plans-for-ocaml-408/":

We plan to change the AST to have an explicit representation for:...

FWIW, I'm not 100% convinced by the proposal, since it would mean treating very differently

let (x1 : t), x2 = e1, e2 in ...

from

let (x1 : t) = x1 in ...

which might be surprising for ppx authors and for users (in the first case, the annotation would not be propagated to the expression e1).

An alternative would be to keep the annotations only in the pattern at the AST level, but somehow extract them (with a strategy to define) and apply them to the bound expression.

github-actions · 2020-05-07T04:16:53Z

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

garrigue · 2020-05-09T09:25:51Z

I vote to close this. Here parentheses are relevant.

gasche · 2020-05-09T12:06:50Z

@garrigue there are different things here. One is a special status of let p : t = .., but the other is whether the propagation of type information happens through a syntactic desugaring in the parsetree, or by some later means.

We can decide that we like or not the special typing discipline in the let p : t case (vs. let (p : t)), but I think it is rather consensual than having this as a parse-time desugaring is not a good approach, especially if it makes the life of ppx authors harder.

github-actions · 2021-05-12T04:27:31Z

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

garrigue · 2021-05-12T06:44:13Z

First, to clarify, originally the two syntaxes were already treated differently.
I.e., before the introduction of polymorphic recursion, (1) was indeed parsed as

let foo = (42 : int) in

which is coherent with the handling in functions:

let foo x : int = x+1

Now, for ppx's, wouldn't it be sufficient to disable the duplication of the type into the pattern when there is an extension?
This would make sense, since the semantics of let is changed, so that x is no longer guaranteed to be of type int (while 42 ought to be).

github-actions · 2022-05-16T04:26:36Z

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

Octachron · 2022-05-16T12:07:09Z

Another reason to have a distinct parsetree node for let p: ty = x in is to remove the use of Ptyp_poly([],_) (see #11148) in the parsetree and remove the code in the parsetree pretty-printer that relies on this encoding. I think it would also simplify the code of the typer in typecore which adds another layer of duplication of constraints.

Octachron · 2024-02-07T16:30:01Z

Since #12119, let pat : ty = expr and let pat [: typ ] :> typ = expr are represented in an optional field of the value binding node of the parsetree.
This issue can thus be closed.

vicuna added the lexing-and-parsing label Mar 14, 2019

gpetiot mentioned this issue Apr 8, 2019

Type annotation on first-class modules ocaml-ppx/ocamlformat#629

Closed

github-actions bot added the Stale label May 7, 2020

gasche removed the Stale label May 9, 2020

github-actions bot added the Stale label May 12, 2021

github-actions bot removed the Stale label May 14, 2021

github-actions bot added the Stale label May 16, 2022

github-actions bot removed the Stale label May 18, 2022

Octachron self-assigned this Feb 23, 2023

Octachron closed this as completed Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pattern type constraint in let expression parsed inconsistently #7758

Pattern type constraint in let expression parsed inconsistently #7758

vicuna commented Mar 25, 2018 •

edited by yallop

vicuna commented Apr 11, 2018

vicuna commented Oct 15, 2018

github-actions bot commented May 7, 2020

garrigue commented May 9, 2020

gasche commented May 9, 2020

github-actions bot commented May 12, 2021

garrigue commented May 12, 2021

github-actions bot commented May 16, 2022

Octachron commented May 16, 2022

Octachron commented Feb 7, 2024

Pattern type constraint in let expression parsed inconsistently #7758

Pattern type constraint in let expression parsed inconsistently #7758

Comments

vicuna commented Mar 25, 2018 • edited by yallop

Bug description

Steps to reproduce

Additional information

vicuna commented Apr 11, 2018

vicuna commented Oct 15, 2018

github-actions bot commented May 7, 2020

garrigue commented May 9, 2020

gasche commented May 9, 2020

github-actions bot commented May 12, 2021

garrigue commented May 12, 2021

github-actions bot commented May 16, 2022

Octachron commented May 16, 2022

Octachron commented Feb 7, 2024

vicuna commented Mar 25, 2018 •

edited by yallop