AST-38: Significant parenthesis around tuple patterns in func arguments by ggreif · Pull Request #178 · caffeinelabs/motoko

ggreif · 2019-02-20T11:25:37Z

This now passes tests, I assume that return types are already fixed by #111.

~~I might remove the new test as it does not add extra value.~~

However now I get an assertion failure with

func g1((),) {}

~~which worries me a bit.~~ This is now AST-56.

and guide it through to type checker. Parser still not done, instead we explicitly write a 1-ary pattern at toplevel.

src/interpret.ml

src/parser.mly

ggreif · 2019-02-20T13:07:54Z

I am sufficiently confident that I am doing the right thing, so let's make this a proper pull request.

Merge the two tests (old one from @nomeata and my new one)?
Investigate assertion failure in wasm (or write story: https://dfinity.atlassian.net/browse/AST-56)
class is a constructor function, so it should inherit the arity semantics from func?
in new test, f(()) is accepted, but f is nullary. Is that okay?
fix parser ambiguities

I'd like to find answers/comments to the above.

rossberg · 2019-02-20T14:48:08Z

Merge the two tests (old one from @nomeata and my new one)?

I suppose so.

Investigate assertion failure in wasm (or write story for it)

Which failure is that?

class is a constructor function, so it should inherit the arity semantics from func?

Correct.

in new test, f(()) is accepted, but f is nullary. Is that okay?

Yes. At calls there is an implicit coercion between tuples and n-ary arguments (same for results). The difference really only matters in higher-order cases, (()) -> () is a different function type than () -> (), and they cannot be coerced.

nomeata · 2019-02-20T14:59:28Z

Investigate assertion failure in wasm (or write story for it)

Which failure is that?

https://github.com/dfinity-lab/actorscript/pull/178/files#diff-52bad8419afa40f606ae789a77d9a65dR1

Ill-typed intermediate code (use -v to see dumped IR):
issue38.as:46.1-46.15: IR type error, subtype violation:
  (((),)) -> ()
  (()) -> ()

Note that “in wasm” is a red herring, it is not about running the wasm, but about compiling, as the error is a type error in the IR.

I need to include the phase in the “issue38.as:46.1-46.15: IR type error, subtype violation:” message to see clearly if this arose after desugaring (indicating a bug in typing, desugar or check_ir), or after a later pass (indicating a bug in that pass or check_ir). Until then, run with -v to see after which phase it occurred.

nomeata · 2019-02-20T15:06:43Z

The IR is already ill-typed after desugaring (the following is the output with #179)

Ill-typed intermediate code after desugaring (use -v to see dumped IR):
issue38.as:46.1-46.15: IR type error, subtype violation:
  (((),)) -> ()
  (()) -> ()

Unrelatedly, the build now says

Warning: one state has reduce/reduce conflicts.
Warning: 3 reduce/reduce conflicts were arbitrarily resolved.

is that something that ought to worry us?

crusso · 2019-02-20T15:11:49Z

Gabor, is you set OCAMLRUNPARAM="b" in your environment, then you should get a stack trace that pinpoints where the error occurred in the IR checker.

crusso · 2019-02-20T15:14:51Z

And yes, parsing ambiguities should be removed, especially reduce/reduce ones.

…ties

ggreif · 2019-02-20T16:05:55Z

parser.conflicts.txt

Working on a fix... Done.

rossberg · 2019-02-20T16:16:15Z

I think the right fix is not to handle the parameter parens case in parsing but in typing.

so that `pat_top` and `pat_nullary` can share the common innards. This avoids the reduce/reduce conflicts reported by `menhir`. CAVEAT: I am not a grammar expert, so there may be better ways to accomplish this. Suggestions welcome.

crusso · 2019-02-20T17:57:36Z

@ggreif Did you get issue38 above working? I checked out the branch but cannot reproduce it....

crusso · 2019-02-20T18:26:14Z

FTR Never mind - the bug is still there, but you need to run the test manually to see it …

test/run/empty-tup-args.as

crusso · 2019-02-27T13:57:09Z

src/desugar.ml

+    | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }
+    | _ -> p in
+  let param p = match p.it with
+    | S.ParP p1 -> pat (fix_unary { p with it = S.TupP [p1]; note = Type.Tup [p.note] })


In the S.ParP p1 case, would it not be make more sense to just continue with pat p1 - or is there something more subtle going on here.

Agreed with @crusso, not sure why there are two levels of special casing.

Will fold. There is nothing subtle going on besides "separation of concerns". See my previous explanation.

crusso · 2019-02-27T14:03:21Z

src/operator.ml

  | T.Word32 -> fun v -> Word32 (fword32 (as_word32 v))
  | T.Word64 -> fun v -> Word64 (fword64 (as_word64 v))
-  | _ -> raise (Invalid_argument "unop")
+  | _ -> raise (Invalid_argument "word_unop")


Are these unrelated changes that snuck into the branch? I wouldn't bother reverting, just wanted to draw your attention...

I would prefer reverting that change, since word_unop is an implementation detail of the module, and only unop is meaningful outside.

I am happy to revert. I did this because there are two raise (Invalid_argument "unop") defaults in that file, and when I made an attempt to always wrap ParP around inner patterns I got this exception. Here is the dump:

switch: [tc] [run] [run-low] [run-ir] [wasm] --- switch.tc (expected) +++ switch.tc (actual) @@ -1,6 +1,4 @@ -switch.as:87.33-87.34: warning, this pattern is never matched -switch.as:75.11-77.2: warning, the cases in this switch do not cover all possible values -switch.as:81.3-81.14: warning, the cases in this switch do not cover all possible values -switch.as:87.3-87.40: warning, the cases in this switch do not cover all possible values -switch.as:92.11-94.2: warning, the cases in this switch do not cover all possible values -switch.as:97.11-99.2: warning, the cases in this switch do not cover all possible values +(unknown location): internal error, Invalid_argument("unop") + +Last environment: +

It is kind-of hard to figure which function threw, when two are throwing the same thing :-)
(Cursory debugging of the issue did not give any helpful clues, only that it is not word_unop who is throwing.)

Anyway, if we go forward (with a distinct PR) to equate unary tuples and values in the parser, then the ParP wrapping will be moot anyway, so I prefer to leave the parser as-is for now.

Also my (not pushed) experiment to always add ParP resulted in a bunch of changed source locations (parens around AltP) making the reporting less precise:

-coverage.as:5.8-5.14: warning, this pattern does not cover all possible values +coverage.as:5.7-5.15: warning, this pattern does not cover all possible values

I could fix that by inheriting the inner srcloc, but I don't see the benefit.

crusso

I've added some comment inline. I still think we are only masking the problem with unit tuples, but for now, I think this is almost good to merge (see comments)

rossberg · 2019-02-27T16:03:57Z

src/desugar.ml

 and dec d = phrase' dec' d
-and dec' at n d = match d with
+and dec' at n d =
+  let fix_unary p = match p.it, p.note with


Hm, it looks fishy to me to only apply such a special case in one place. If the solution is to essentially remove unary tuples then the right place to do this normalisation is in the parser, in the productions for tuples, so that the AST never contains a unary tuple type/exp/pat in the first place.

When I wrote this, my intention was not to equate values with the unary tuple formed from them. I was bending to the (empirical) observation that the later stages (i.e. coverage analyser, interpreter) seem to insist that single arguments should not be wrapped by an extra layer of TupP. Please note that at this point both the type system as well as the runtime values do have the notion of unary tuple, the only place where I strip them is here for unary function arguments, with the fix_unary function. The separation I chose arose from the consideration, that I wanted to make clear that we first lower to a canonical form (i.e. TupP around arguments) and then fix the single argument case that causes trouble downstream. As already discussed with @crusso I plan to write a new story for cleaning up the status quo. For this PR I'll fold the two levels into one.

rossberg · 2019-02-27T16:05:22Z

src/desugar.ml

+  let fix_unary p = match p.it, p.note with
+    | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }
+    | _ -> p in
+  let param p = match p.it with


Nit: avoid nesting functions unless they actually close over something.

Okay, this seems to be a stylistic issue. Are there any such conventions written down somewhere? My notion used to be "define utilities in the closest possible scope", which I do here. I'll try to lift this out to top-level when folding the two functions into one. I wonder however, if that will export the function from the module?

rossberg · 2019-02-27T16:08:14Z

src/desugar.ml

+    | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }
+    | _ -> p in
+  let param p = match p.it with
+    | S.ParP p1 -> pat (fix_unary { p with it = S.TupP [p1]; note = Type.Tup [p.note] })


Agreed with @crusso, not sure why there are two levels of special casing.

rossberg · 2019-02-27T16:09:48Z

src/operator.ml

  | T.Word32 -> fun v -> Word32 (fword32 (as_word32 v))
  | T.Word64 -> fun v -> Word64 (fword64 (as_word64 v))
-  | _ -> raise (Invalid_argument "unop")
+  | _ -> raise (Invalid_argument "word_unop")


I would prefer reverting that change, since word_unop is an implementation detail of the module, and only unop is meaningful outside.

ggreif

This is getting waaay bigger than originally thought. I have motivated my decisions to my best knowledge, please ask if something is still unclear. This is on my plate:

revert the unop (and friends) change
fold fix_unary into param and move to toplevel
(optionally) investigate the root cause for the internal error when ParP wraps an AnnP (if you insist)
write follow-on story to rectify the single-argument mess.

ggreif · 2019-02-28T10:29:06Z

src/desugar.ml

 and dec d = phrase' dec' d
-and dec' at n d = match d with
+and dec' at n d =
+  let fix_unary p = match p.it, p.note with


When I wrote this, my intention was not to equate values with the unary tuple formed from them. I was bending to the (empirical) observation that the later stages (i.e. coverage analyser, interpreter) seem to insist that single arguments should not be wrapped by an extra layer of TupP. Please note that at this point both the type system as well as the runtime values do have the notion of unary tuple, the only place where I strip them is here for unary function arguments, with the fix_unary function. The separation I chose arose from the consideration, that I wanted to make clear that we first lower to a canonical form (i.e. TupP around arguments) and then fix the single argument case that causes trouble downstream. As already discussed with @crusso I plan to write a new story for cleaning up the status quo. For this PR I'll fold the two levels into one.

ggreif · 2019-02-28T10:32:54Z

src/desugar.ml

+  let fix_unary p = match p.it, p.note with
+    | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }
+    | _ -> p in
+  let param p = match p.it with


Okay, this seems to be a stylistic issue. Are there any such conventions written down somewhere? My notion used to be "define utilities in the closest possible scope", which I do here. I'll try to lift this out to top-level when folding the two functions into one. I wonder however, if that will export the function from the module?

ggreif · 2019-02-28T10:33:21Z

src/desugar.ml

+    | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }
+    | _ -> p in
+  let param p = match p.it with
+    | S.ParP p1 -> pat (fix_unary { p with it = S.TupP [p1]; note = Type.Tup [p.note] })


Will fold. There is nothing subtle going on besides "separation of concerns". See my previous explanation.

ggreif · 2019-02-28T10:53:27Z

src/parser.mly

    { LitP(ref l) @! at $sloc }
  | LPAR p=pat RPAR
-    { p }
+    { match p.it with


I tried to get away with the distinction, but it caused the

+(unknown location): internal error, Invalid_argument("unop")

failure. I have analysed it and it originates from:

let x2 : Int = switch (-3) { case (0) 0; case (-1) 2; // <<< problem case (-3) 1; case (x) x; };

so the unop is -. The switch expression case legs end up with (ParP ...). There is a bug somewhere handling this case. Seems like unop does not handle Type.Pre.
Note that the parse tree now contains all the extra ParPs:

(LetD (VarP x2) (AnnotE (SwitchE (UnE ??? NegOp (LitE (PreLit 3 Nat))) (case (ParP (LitP (PreLit 0 Nat))) (LitE (PreLit 0 Nat))) (case (ParP (SignP NegOp (PreLit 1 Nat))) (LitE (PreLit 2 Nat))) (case (ParP (SignP NegOp (PreLit 3 Nat))) (LitE (PreLit 1 Nat))) (case (ParP (VarP x)) (VarE x)) ) (VarT Int) ) )

Update: cracked it. infer_pat' should always call infer_pat when descending, but I was calling infer_pat' from the PatP case, thus not updating references.

~~Also, in the desugarer I am assuming that ParP only wraps (TupP ...). See the param (and fix_unary conversation above).~~ The new param is indifferent.

The other reason for not using { ParP p @! at $sloc } is the loss of precision when pointing out non-coverage warnings. I explained this in my previous comment.

and move to toplevel

This reverts commit 9c29a48.

by calling `infer_pat`, so that the note gets updated for the subpattern

now that we correctly typecheck it.

crusso · 2019-03-05T17:10:31Z

src/desugar.ml

+and param p =
+  pat (match p.it, p.note with
+       | S.ParP p1, _ -> p1
+       | S.TupP [p1], Type.Tup [n] -> { p with it = p1.it; note = n }


To this is normalizing patterns on unary tuples to unary arguments - which is different from what Andreas was suggesting, but I'm ok with that for now.

As there will be a bigger clean-up AST soon, I'll defer to that.

crusso · 2019-03-05T17:13:01Z

test/run/empty-tup-args.as

+h1(());
+gh[0](());
+gh[1](());
+gh[2](());


nice tests! Would it make sense to add a couple of negative ones in test/fail too, eg distinguishing unary functions on pairs from binary functions etc

I'll do a little follow-on for this, as I actually have a few stashed tests that fail lying around.

crusso

I'm good with this. I suggested added some negative tests but I'll leave compliance up to you. Sorry I took so long to review.

## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@406decfa...a1b3f670](dfinity/ic-hs@406decf...a1b3f67) * [`d4db8d07`](dfinity/ic-hs@d4db8d0) bump nixpkgs to 7c786944f801745310578d1cfc019923396f830c ([dfinity/ic-hs⁠#163](https://github.com/dfinity/ic-hs/issues/163)) * [`31d535d2`](dfinity/ic-hs@31d535d) increase the number of allowed delegations in a request from 4 to 20 ([dfinity/ic-hs⁠#166](https://github.com/dfinity/ic-hs/issues/166)) * [`4a310c0d`](dfinity/ic-hs@4a310c0) support ic0.is_controller ([dfinity/ic-hs⁠#169](https://github.com/dfinity/ic-hs/issues/169)) * [`5fc27bdc`](dfinity/ic-hs@5fc27bd) do not include keep-alive header in httpbin response ([dfinity/ic-hs⁠#170](https://github.com/dfinity/ic-hs/issues/170)) * [`96448083`](dfinity/ic-hs@9644808) drop nix-build-uncached ([dfinity/ic-hs⁠#175](https://github.com/dfinity/ic-hs/issues/175)) * [`c6fbe1f7`](dfinity/ic-hs@c6fbe1f) increase ingress_expiry in reference test suite ([dfinity/ic-hs⁠#176](https://github.com/dfinity/ic-hs/issues/176)) * [`a1b3f670`](dfinity/ic-hs@a1b3f67) add Connection: close header to httpbin ([dfinity/ic-hs⁠#178](https://github.com/dfinity/ic-hs/issues/178))

## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@406decfa...9152a0ff](dfinity/ic-hs@406decf...9152a0f) * [`d4db8d07`](dfinity/ic-hs@d4db8d0) bump nixpkgs to 7c786944f801745310578d1cfc019923396f830c ([dfinity/ic-hs⁠#163](https://github.com/dfinity/ic-hs/issues/163)) * [`31d535d2`](dfinity/ic-hs@31d535d) increase the number of allowed delegations in a request from 4 to 20 ([dfinity/ic-hs⁠#166](https://github.com/dfinity/ic-hs/issues/166)) * [`4a310c0d`](dfinity/ic-hs@4a310c0) support ic0.is_controller ([dfinity/ic-hs⁠#169](https://github.com/dfinity/ic-hs/issues/169)) * [`5fc27bdc`](dfinity/ic-hs@5fc27bd) do not include keep-alive header in httpbin response ([dfinity/ic-hs⁠#170](https://github.com/dfinity/ic-hs/issues/170)) * [`96448083`](dfinity/ic-hs@9644808) drop nix-build-uncached ([dfinity/ic-hs⁠#175](https://github.com/dfinity/ic-hs/issues/175)) * [`c6fbe1f7`](dfinity/ic-hs@c6fbe1f) increase ingress_expiry in reference test suite ([dfinity/ic-hs⁠#176](https://github.com/dfinity/ic-hs/issues/176)) * [`a1b3f670`](dfinity/ic-hs@a1b3f67) add Connection: close header to httpbin ([dfinity/ic-hs⁠#178](https://github.com/dfinity/ic-hs/issues/178)) * [`d3812ffc`](dfinity/ic-hs@d3812ff) increase delegation expiry in tests ([dfinity/ic-hs⁠#182](https://github.com/dfinity/ic-hs/issues/182)) * [`40a46e2f`](dfinity/ic-hs@40a46e2) sync universal-canister with IC repo ([dfinity/ic-hs⁠#177](https://github.com/dfinity/ic-hs/issues/177)) * [`7a6259c2`](dfinity/ic-hs@7a6259c) decrease number of threads and request submission latency ([dfinity/ic-hs⁠#179](https://github.com/dfinity/ic-hs/issues/179)) * [`a9f73dba`](dfinity/ic-hs@a9f73db) fix decoding compressed WASM modules during snapshotting ([dfinity/ic-hs⁠#184](https://github.com/dfinity/ic-hs/issues/184)) * [`64c19a95`](dfinity/ic-hs@64c19a9) bump nixpkgs to eaf03591711b46d21abc7082a8ebee4681f9dbeb ([dfinity/ic-hs⁠#189](https://github.com/dfinity/ic-hs/issues/189)) * [`9152a0ff`](dfinity/ic-hs@9152a0f) add date header to httpbin responses and make http header names lower-case ([dfinity/ic-hs⁠#188](https://github.com/dfinity/ic-hs/issues/188)) Includes and closes #3915. Reason: `ic-hs` and `nixpkgs` must be in sync, so that the artefact caching can work.

ggreif added 5 commits February 19, 2019 11:21

AST-38: add failing test

08e32c3

add Int arguments

921c06a

AST-38: introduce ParP to indicate unary function

7fcb0a5

and guide it through to type checker. Parser still not done, instead we explicitly write a 1-ary pattern at toplevel.

no more type errors, TODO: non-fail

67135be

fix parser to remember significant parenthesis

3d3ee75

ggreif requested a review from crusso February 20, 2019 11:34

test successful

4fe6e09

rossberg reviewed Feb 20, 2019

View reviewed changes

src/interpret.ml Outdated Show resolved Hide resolved

src/parser.mly Outdated Show resolved Hide resolved

ggreif added 2 commits February 20, 2019 13:13

beef up the testing, discovering a subtle issue in wasm

2d8eac1

review feedback

1383827

ggreif marked this pull request as ready for review February 20, 2019 13:04

ggreif requested a review from nomeata February 20, 2019 13:08

ggreif added Bug Something isn't working starter project Good for newcomers labels Feb 20, 2019

classes are basically functions and as such inherit the top-level ari…

3f2e405

…ties

Factor out pat_simple

da9c08b

so that `pat_top` and `pat_nullary` can share the common innards. This avoids the reduce/reduce conflicts reported by `menhir`. CAVEAT: I am not a grammar expert, so there may be better ways to accomplish this. Suggestions welcome.

ggreif added 3 commits February 21, 2019 16:27

more tests (classes)

0d797e6

consolidate tests

53cb2ad

fix switch syntax in README

d31dcac

nomeata reviewed Feb 21, 2019

View reviewed changes

test/run/empty-tup-args.as Show resolved Hide resolved

nomeata removed their request for review February 26, 2019 11:19

crusso reviewed Feb 27, 2019

View reviewed changes

rossberg reviewed Feb 27, 2019

View reviewed changes

ggreif commented Feb 28, 2019

View reviewed changes

ggreif added 7 commits February 28, 2019 13:12

review feedback: fold 'fix_unary' into 'param'

92ea6b6

and move to toplevel

Revert "more precise Invalid_argument messages"

c67a012

This reverts commit 9c29a48.

Merge remote-tracking branch 'origin/master' into gabor/one-tuples

1052fbb

transform FuncE parameters in exp'

f93ecdc

extend tests to include function expressions

bffcea3

fix infer_pat' descent into PatP

e4e7268

by calling `infer_pat`, so that the note gets updated for the subpattern

experimental: always create ParP

c392ade

now that we correctly typecheck it.

ggreif requested review from crusso, nomeata and rossberg March 5, 2019 09:10

crusso reviewed Mar 5, 2019

View reviewed changes

crusso approved these changes Mar 5, 2019

View reviewed changes

ggreif mentioned this pull request Mar 5, 2019

accept decl-clash.*.ok #215

Merged

review feedback: add negative tests

81200bb

ggreif merged commit cd26cd3 into master Mar 5, 2019

ggreif deleted the gabor/one-tuples branch March 5, 2019 22:42

Conversation

ggreif commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ggreif commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rossberg commented Feb 20, 2019

Uh oh!

nomeata commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomeata commented Feb 20, 2019

Uh oh!

crusso commented Feb 20, 2019

Uh oh!

crusso commented Feb 20, 2019

Uh oh!

ggreif commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rossberg commented Feb 20, 2019

Uh oh!

crusso commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crusso commented Feb 20, 2019

Uh oh!

Uh oh!

crusso Feb 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crusso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreif left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreif Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreif commented Feb 20, 2019 •

edited

Loading

ggreif commented Feb 20, 2019 •

edited

Loading

nomeata commented Feb 20, 2019 •

edited

Loading

ggreif commented Feb 20, 2019 •

edited

Loading

crusso commented Feb 20, 2019 •

edited

Loading

crusso Feb 27, 2019 •

edited

Loading

ggreif left a comment •

edited

Loading

ggreif Feb 28, 2019 •

edited

Loading