Fix bug in untypeast/pprintast for value bindings with polymorphic type annotations - #13845
Conversation
| let pat = sub.pat sub vb.vb_pat in | ||
| let pat, value_constraint = | ||
| match pat.ppat_desc with | ||
| | Ppat_constraint (pat, cty) -> |
There was a problem hiding this comment.
I would suggest to apply this transformation only in the Ptyp_poly case to make the parsetree -> typedtree -> parsetree transformation idempotent. Otherwise, we are duplicating the constraint on the expression side.
There was a problem hiding this comment.
Makes sense - I've specialized the case to apply only when cty is a Ptyp_poly.
| (***********************************) | ||
| (* Untypeast/pprintast correctly handle value binding type annotations. *) | ||
|
|
||
| run {| let foo : 'a. 'a -> 'a = fun x -> x in foo |};; |
There was a problem hiding this comment.
The parser also supports let foo : type a . a -> a = fun x -> x, does this work as expected?
There was a problem hiding this comment.
This syntax is elaborated in Typecore to
let (foo: 'a. 'a -> 'a) = fun (type a) -> ( (fun x -> x) : 'a -> 'a)and it is thus is similar to the first form in the typedtree.
There was a problem hiding this comment.
I've added this example as a test case. It prints as @Octachron predicts.
It is expected that you can get back a valid source program if you typecheck a program, then run
untypeaston the resulting typedtree, then print it withpprintast(see tests of this intestsuite/tests/compiler-libs/test_untypeast.ml). However, this does not work for the following expression:The issue is that
untypeastturns the pattern in the let binding into aPpat_constraintnode, rather than putting the type annotation in thepvb_constraintfield. As a result, this invalid syntax is printed:This PR has two commits. The first demonstrates this issue by adding this testcase to
test_untypeast.ml. The second fixes it, by simply havinguntypeastcheck whether the pattern in a value binding is aPpat_constraint, and moving the type annotation to thepvb_constraintfield in that case.