Conversation
…ion arguments; needs refactoring
Not sure I fully follow. Is there an example for that bug? |
nomeata
left a comment
There was a problem hiding this comment.
Looks, good, only small suggestions!
Yes, I've checked this in as test\bugs\aritybug.as: |
|
Oh, we actually have syntax for unary tuples? I thought we did not, and hence assumed this problem would not Do we need syntax for unary tuples? Is the problem due to the domain or the codomain of the function, or does it occur in both? I think the solution is the following: When determining the arity of a function based on the syntactic form of the declaration, we currently apply these rules:
I think the following rules will resolve the issue:
This enforces that we will have I guess one “other way” is to make |
|
Yes, I was thinking along those lines too. We just want to ensure that the encoding doesn't identify types that should be kept distinct. On the other hand, unary tuples seems useless to me thought the unformity is nice. |
|
When compiling Ah, no, I guess it only checks that the calling conventions match, but it still happily passes values that don't fit the calling convention. So it looks like there is a type error in the translation somewhere. |
|
Ok, indeed the interpreter sees the problem. On my branch for n-ary function calls, I extended the interpreter to check this (I will remove this before merging, as it is likely too much debugging related cdoe for Andreas’ taste, but until the bug is fixes this is useful). You can get the check onto your own branch using It only triggers in so you are trying to pass a single |
nomeata
left a comment
There was a problem hiding this comment.
Let’s make sure not only the calling conventions match, but also the actual arguments match the calling convention.
…lwith by assert false; code formatting
|
Bugger, I'll investigate. |
|
Investigating a bit myself.
In the second and third case, the type checker gives this type to the function: I believe the problem is rooted somewhere deeper than your translation, and is related to the confusion around unary tuples, function taking no tuples, etc. Let's discuss that at #113 and then come back here. |
|
I'm having more and more second thoughts on the whole n-ary function type thing. It has become rather complicated and its failure to represent the unary tuple type consistently is a red flag as well. I wish I had thought of that earlier. We had considered the flag alternative before, which would not have this problem, though perhaps others. Here is a third, maybe more systematic idea: we introduce an Unboxed type constructor. Then The Unboxed constructor of course is internal and can only be applied to types of internal kind Unboxable (which, for now, is the supertype of all tuple types -- edit: and of Async.). An Unboxed type itself has a kind that may only occur as arrow parameter result or as Async operand. I believe this would avoid any discontinuity in the type algebra and be properly compositional. (Also related to #113.) |
|
Agreed that this became a mess. When you write And will it be the case that Is there a semantic difference to the boolean flag approach (besides different trade-offs in implementation and data representation)? The argument to a function can be unary (no Sounds all sound and doable. What I don’t understand yet: Why do we fail do to the following today properly: I obtained this by composing your translation with this one (which is the translation the compiler would do): In other words: What is the crucial property that makes the |
It's concrete syntax, with
Yes.
I assume the end result is the same, but I think it factors it a bit nicer and less ad-hoc.
The difference is that (a) no case distinction is needed, making it uniform and avoiding compositionality hazards, and (b) we don't lose the information like your extra rule does, which fails to distinguish e.g. Unboxed (t,) from t. |
|
Note that this extends to async: a type |
|
Sound good, ship it :-) |
|
In fact, we could use In other words, when This is just a glimpse of what we can do down the road eventually … but I agree that this goes in a good direction. |
What about the term level: Would there be |
|
I agree that we should have those on the term level. However, the question then is where to introduce them. The type ones could be introduced as desugarings in the parser already. But for the term level these are essentially implicit conversions that can only be introduced after typing. Maybe we'll need to introduce a notion of implicit coercions after all? |
|
I still think we are overthinking this. If we had simple type and syntax based distinction between second class sequences and first-class tuples we would be done. Functions takes sequence patterns and return sequences, expressions produce sequences and tuple constructors take sequences and produce a singleton sequence that contains a tuple.Variables are never bound to sequences, only types, and functions are always applied to sequences and return sequences. Type range over types, not sequences. This is precisely what Kevin Mitchell does in his paper. The current approach is flawed because we are trying to recover sequences from types that can be both non-tuples and tuples, blurring the distinction between plain values and singleton tuples. |
|
I would expect the type inference to add the terms level Claudio, it seems that |
|
Yes, I would just make changes to the syntax instead of being clever about it. JS users are familiar with parenthesized parameter sequences, AFAIK, not juxtaposition. So let's adopt that and add something like It seems to me that Unboxed is taking an arbitrary type, not a sequence of types - feels more like a type destructor than constructor to me ;-> but perhaps I'm just misreading it. |
…in the intepreter. Shared function bodies must be rewritten taking the syntactic form of the argument pattern (TupP or not) into account, not just the domain type. Calls to awaitable functions must be rewritten guided by the domain (ie. sequence type) of the called function.
discovered lurking bugs in both translation due to misconception that LetD(pat,exp) has type of exp, not unit as the interpreter assumes. Now fixed. I suspect the Await translation of ObjE isn't quite right regarding scoping of this. Will revisit later.
...no specific reason, but it's fun! Ge get ``` $ git log --oneline --first-parent 8c07b4a592e7c54ff43adf0420575d4069bfe8a9..233f63f23f0a207f291693fc1983a92a53e28b59 ``` 233f63f (HEAD -> master, origin/master, origin/HEAD) Merge pull request #111 from dfinity-lab/nm-update-naersk acb95af Merge pull request #110 from dfinity-lab/nm-force-docheck 6b8ecf7 Merge pull request #108 from dfinity-lab/basvandijk/fetch-sources.nix 1b0691a Merge pull request #109 from dfinity-lab/basvandijk/filter-dot-git-from-nix-fmt f0db4f4 Merge pull request #107 from dfinity-lab/nm-cmake-bash
## Changelog for candid: Branch: Commits: [dfinity/candid@a1dcbad4...3e3ad95a](dfinity/candid@a1dcbad...3e3ad95) * [`119703ba`](dfinity/candid@119703b) [Spec] Relax LEB128 decoding ([dfinity/candid#79](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/79)) * [`10f08432`](dfinity/candid@10f0843) Update prim.test.did * [`b2524816`](dfinity/candid@b252481) parser for test suite ([dfinity/candid#78](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/78)) * [`71bf6e76`](dfinity/candid@71bf6e7) release 0.5.2 ([dfinity/candid#80](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/80)) * [`b9f387e3`](dfinity/candid@b9f387e) test suite for JS ([dfinity/candid#81](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/81)) * [`9e5dc775`](dfinity/candid@9e5dc77) Release ([dfinity/candid#82](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/82)) * [`1df9d2d7`](dfinity/candid@1df9d2d) more candid test data ([dfinity/candid#83](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/83)) * [`9e4156d9`](dfinity/candid@9e4156d) fix newtype ([dfinity/candid#85](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/85)) * [`6880a430`](dfinity/candid@6880a43) display for types ([dfinity/candid#86](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/86)) * [`04b1b068`](dfinity/candid@04b1b06) release ([dfinity/candid#87](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/87)) * [`117c6436`](dfinity/candid@117c643) Refactor Lexer ([dfinity/candid#89](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/89)) * [`0a5789f9`](dfinity/candid@0a5789f) fix value pretty printer ([dfinity/candid#92](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/92)) * [`9f35a5aa`](dfinity/candid@9f35a5a) refactor error ([dfinity/candid#94](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/94)) * [`2e742927`](dfinity/candid@2e74292) Parse annvals in textual format ([dfinity/candid#93](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/93)) * [`0a144c79`](dfinity/candid@0a144c7) use principal from ic-types ([dfinity/candid#84](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/84)) * [`01412b14`](dfinity/candid@01412b1) release ([dfinity/candid#95](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/95)) * [`f540df54`](dfinity/candid@f540df5) release ([dfinity/candid#98](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/98)) * [`798675d8`](dfinity/candid@798675d) Add generic functions to encode/decode around a tuple ([dfinity/candid#99](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/99)) * [`0d26e568`](dfinity/candid@0d26e56) release ([dfinity/candid#100](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/100)) * [`191b6f1f`](dfinity/candid@191b6f1) Reset record_nesting_depth after each value ([dfinity/candid#101](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/101)) * [`8e7be65d`](dfinity/candid@8e7be65) fix record ([dfinity/candid#103](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/103)) * [`159533b2`](dfinity/candid@159533b) Update construct.test.did * [`a6ea0991`](dfinity/candid@a6ea099) add service initialization parameters ([dfinity/candid#88](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/88)) * [`3a1f56fa`](dfinity/candid@3a1f56f) refactor: sort dependencies and add traits for error types ([dfinity/candid#105](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/105)) * [`89df78ee`](dfinity/candid@89df78e) support service constructor ([dfinity/candid#106](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/106)) * [`738d5ed4`](dfinity/candid@738d5ed) fix for actor class codegen ([dfinity/candid#107](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/107)) * [`97ba7a0f`](dfinity/candid@97ba7a0) export init args in js ([dfinity/candid#108](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/108)) * [`d4e00adc`](dfinity/candid@d4e00ad) fix js init export ([dfinity/candid#109](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/109)) * [`c1662abe`](dfinity/candid@c1662ab) [spec] Reverse subtyping ([dfinity/candid#110](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/110)) * [`713595be`](dfinity/candid@713595b) The “reverse variant extension rule” is redundand ([dfinity/candid#113](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/113)) * [`79d49a01`](dfinity/candid@79d49a0) Spec: Opt decoding also from non-opt values ([dfinity/candid#114](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/114)) * [`2cfc0ecf`](dfinity/candid@2cfc0ec) improve pretty printing for values ([dfinity/candid#116](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/116)) * [`8fafe345`](dfinity/candid@8fafe34) Un-rename Soundness document ([dfinity/candid#115](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/115)) * [`8e6fc502`](dfinity/candid@8e6fc50) Bump spec version ([dfinity/candid#112](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/112)) * [`7cedebcb`](dfinity/candid@7cedebc) fix clippy ([dfinity/candid#117](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/117)) * [`a732a639`](dfinity/candid@a732a63) Candid UI Canister ([dfinity/candid#111](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/111)) * [`d97b271c`](dfinity/candid@d97b271) disable pretty printing for large vectors ([dfinity/candid#118](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/118)) * [`aceb7f92`](dfinity/candid@aceb7f9) derive candid type for functions ([dfinity/candid#119](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/119)) * [`c3dc0ad7`](dfinity/candid@c3dc0ad) rename derived code for CDK ([dfinity/candid#120](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/120)) * [`d1f8de7d`](dfinity/candid@d1f8de7) release ([dfinity/candid#121](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/121)) * [`3e3ad95a`](dfinity/candid@3e3ad95) remove multi-line string in test suites ([dfinity/candid#125](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity/candid/issues/125))
## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@0f87c270...20e23e1a](dfinity/ic-hs@0f87c27...20e23e1) * [`8c2207ee`](dfinity/ic-hs@8c2207e) query balance and balance128 within the same query ([dfinity/ic-hs#106](https://github.com/dfinity/ic-hs/issues/106)) * [`5a076ad1`](dfinity/ic-hs@5a076ad) rename canister state counter to canister version ([dfinity/ic-hs#109](https://github.com/dfinity/ic-hs/issues/109)) * [`d095478b`](dfinity/ic-hs@d095478) Include ic-ref-test and universal canister in release build artifacts ([dfinity/ic-hs#111](https://github.com/dfinity/ic-hs/issues/111)) * [`3a58e164`](dfinity/ic-hs@3a58e16) Add canister global timer ([dfinity/ic-hs#107](https://github.com/dfinity/ic-hs/issues/107)) * [`c863b6ef`](dfinity/ic-hs@c863b6e) fix provisional top up test ([dfinity/ic-hs#112](https://github.com/dfinity/ic-hs/issues/112)) * [`2cd76efb`](dfinity/ic-hs@2cd76ef) run system tasks periodically ([dfinity/ic-hs#113](https://github.com/dfinity/ic-hs/issues/113)) * [`3c2eb69a`](dfinity/ic-hs@3c2eb69) narrow down gap for canister http_requests between ic-ref(-test) and Interface Spec ([dfinity/ic-hs#100](https://github.com/dfinity/ic-hs/issues/100)) * [`fb76b646`](dfinity/ic-hs@fb76b64) ic-ref-run: Execute heartbeats before any submitted message ([dfinity/ic-hs#114](https://github.com/dfinity/ic-hs/issues/114)) * [`20e23e1a`](dfinity/ic-hs@20e23e1) do not return an HTTP error for calls to stopping/stopped canisters ([dfinity/ic-hs#115](https://github.com/dfinity/ic-hs/issues/115))
## Changelog for ic-hs: Branch: master Commits: [dfinity/ic-hs@0f87c270...20e23e1a](dfinity/ic-hs@0f87c27...20e23e1) * [`8c2207ee`](dfinity/ic-hs@8c2207e) query balance and balance128 within the same query ([dfinity/ic-hs#106](https://github.com/dfinity/ic-hs/issues/106)) * [`5a076ad1`](dfinity/ic-hs@5a076ad) rename canister state counter to canister version ([dfinity/ic-hs#109](https://github.com/dfinity/ic-hs/issues/109)) * [`d095478b`](dfinity/ic-hs@d095478) Include ic-ref-test and universal canister in release build artifacts ([dfinity/ic-hs#111](https://github.com/dfinity/ic-hs/issues/111)) * [`3a58e164`](dfinity/ic-hs@3a58e16) Add canister global timer ([dfinity/ic-hs#107](https://github.com/dfinity/ic-hs/issues/107)) * [`c863b6ef`](dfinity/ic-hs@c863b6e) fix provisional top up test ([dfinity/ic-hs#112](https://github.com/dfinity/ic-hs/issues/112)) * [`2cd76efb`](dfinity/ic-hs@2cd76ef) run system tasks periodically ([dfinity/ic-hs#113](https://github.com/dfinity/ic-hs/issues/113)) * [`3c2eb69a`](dfinity/ic-hs@3c2eb69) narrow down gap for canister http_requests between ic-ref(-test) and Interface Spec ([dfinity/ic-hs#100](https://github.com/dfinity/ic-hs/issues/100)) * [`fb76b646`](dfinity/ic-hs@fb76b64) ic-ref-run: Execute heartbeats before any submitted message ([dfinity/ic-hs#114](https://github.com/dfinity/ic-hs/issues/114)) * [`20e23e1a`](dfinity/ic-hs@20e23e1) do not return an HTTP error for calls to stopping/stopped canisters ([dfinity/ic-hs#115](https://github.com/dfinity/ic-hs/issues/115))
(NB: there is a lingering bug (in the typechecker) due to functions taking unary tuples receiving the same flattened type as a function taking a single argument, still working on a fix but I think the correct thing to do is to either revamp the type-checker to deal with n-ary args/results or simply rule out unary tuples for now)