More clarifications and tweaks #91

rossberg · 2020-05-28T14:11:27Z

Following implementation feedback from @lukewagner, @jakobkummerow, and @tebbi, includes the following changes:

RTT types include a natural number tracking their inheritance depth, in order to actually enable efficient casts.
Disallow struct/array.new to take a super-RTT, in order to allow direct representation of RTTs as map/shape objects without hidden dynamic allocation. We could still provide partial type hiding via another RTT allocation instruction, but that can be deferred.
Fully type-annotate RTT-related instructions for now.
Renamed constype to heaptype, as in funcref repo.

More to come...

rossberg · 2020-05-28T14:12:24Z

@jakobkummerow, PTAL. For some reason, GH doesn't allow me to add you as a reviewer.

binji · 2020-05-28T16:41:42Z

@jakobkummerow, PTAL. For some reason, GH doesn't allow me to add you as a reviewer.

I've added him as a contributor, you should be able add him after he accepts the invitation.

taralx · 2020-05-30T21:22:58Z

proposals/gc/MVP.md

-* `i31` is a new constructed type
-  - `constype ::= ... | 31`
+* `i31` is a new heap type
+  - `heaptype ::= ... | 31`


taralx · 2020-05-30T21:27:24Z

The array.new* instructions still accept a super-RTT, is that intentional?

rossberg · 2020-06-02T12:18:25Z

Thanks @taralx, both fixed.

taralx · 2020-06-02T16:28:38Z

proposals/gc/MVP.md

-* `rtt.canon <constype>` returns the RTT of the specified type
-  - `rtt.canon t : [] -> [(rtt t)]`
+* `rtt.canon <heaptype>` returns the RTT of the specified type
+  - `rtt.canon t : [] -> [(rtt 0 t)]`
  - multiple invocations of this instruction yield the same observable RTTs
  - this is a *constant instruction*
  - equivalent to `(rtt.sub t (rtt.canon any))`, except when `t` itself is `any`


What is the type of (rtt.canon any)? Does n go negative, or is the typing of rtt.sub different for any?

Ah, good catch! Wrt handling any this was off by one. Fixed rule and description to something more sensible.

This line is also missing the <n> update to rtt.sub. Also, rtt.sub n t1 t2 wants t1 to be the supertype. So I think it should be (rtt.sub 1 (rtt.canon any) t).

Ah yes, fixed (it's (rtt.sub 1 any t (rtt.canon any))).

jakobkummerow

Looks good to me.

jakobkummerow · 2020-06-03T15:24:01Z

proposals/gc/MVP.md

-* `rtt.canon <constype>` returns the RTT of the specified type
-  - `rtt.canon t : [] -> [(rtt t)]`
+* `rtt.canon <heaptype>` returns the RTT of the specified type
+  - `rtt.canon t : [] -> [(rtt 0 t)]`
  - multiple invocations of this instruction yield the same observable RTTs
  - this is a *constant instruction*
  - equivalent to `(rtt.sub t (rtt.canon any))`, except when `t` itself is `any`


This line is also missing the <n> update to rtt.sub. Also, rtt.sub n t1 t2 wants t1 to be the supertype. So I think it should be (rtt.sub 1 (rtt.canon any) t).

jakobkummerow · 2020-06-03T15:25:27Z

proposals/gc/MVP.md

@@ -186,34 +203,32 @@ Perhaps add the following short-hands:
 * `struct.new <typeidx>` allocates a structure of type `$t` and initialises its fields with given values


Didn't we talk about dropping the rtt-less variants of struct.new? Am I misremembering, or have you changed your mind, or is that postponed until after further/wider discussion?

jakobkummerow · 2020-06-03T15:32:07Z

proposals/gc/MVP.md

    - iff `$t = struct (mut t)*`
-    - and `(type $t) <: t'`
+    - and `(type $t) == t'`


I'm confused by the (type $t) part. Isn't $t a type? What does (type ...) do? Also, I assume == is equivalent to = in the line above? I would have expected to see just $t = t' here, at which point we might as well drop t' and use $t directly instead: (rtt n $t)

If I get to make further wishes: I find the t* part confusing too, because that's not a repeated sequence of a single t type. Maybe at least add an explanation "t* means a sequence of stack values whose types match the struct definition of $t"? Or rephrase the formal definition as something like:

struct.new_with_rtt $t : [(rtt n $t) x y z...] -> [(ref $t)] iff $t = (struct (mut x) (mut y) (mut z)...)

(which would have the additional benefit that it makes more obvious that the value for the last field is the topmost stack value -- which is arguably intuitive but doesn't hurt to spell out explicitly).

I'm confused by the (type $t) part. Isn't $t a type?

It's a "type use" referring to $t, see the new definition of heap types. The extra keyword avoids ambiguity in some syntactic contexts and is more consistent with other syntax. Note that (ref $t) is defined as a shorthand for (ref (type $t)).

FWIW, I just removed the type annotation on the instruction, because with type equivalence it can be derived from the operand type.

If I get to make further wishes: I find the t* part confusing too, because that's not a repeated sequence of a single t type.

It's a sequence of possibly different value types. It's a notational convention that is used heavily in the spec, explained here.

I just removed the type annotation on the instruction, because with type equivalence it can be derived from the operand type.

I generally agree with removing the redundancy; but I think the current specification [(rtt n t) t'*] isn't implementable: we can't find the rtt on the value stack if we don't know how many t' there are, which in turn we can only determine from the rtt. I can see several solutions:
(1) go back to having a type immediate
(2) add an arity immediate (equalling n, indicating the number of t' stack values)
(3) swap the order of the stack arguments to [t'* (rtt n t)]

Ah, yes, good catch, that's ambiguous. I reinstated the type immediate. I figure that if we want to remove it, we should do so consistently along with the other instructions anyway.

jakobkummerow · 2020-06-03T15:32:46Z

proposals/gc/MVP.md

+```
+(type $A (struct))
+(type $B (struct (field i32)))
+(type $C (struct (field i32 i64)))


I'd suggest to abbreviate a little less: (struct (field i32) (field i64))

jakobkummerow · 2020-06-03T15:41:19Z

proposals/gc/MVP.md

  - multiple invocations of this instruction with the same operand yield the same observable RTTs
  - this is a *constant instruction*

+TODO: Add the ability to generate new (non-canonical) RTT values to implement casting in nominal type hierarchies.


Do you just want to go ahead and add rtt.fresh which is the same as rtt.sub except for the line "multiple invocations ..."?

Not yet, it may be unnecessary in the presence of nominal types.

rossberg

Thanks for the close look!

rossberg · 2020-06-04T12:16:19Z

proposals/gc/MVP.md

@@ -186,34 +203,32 @@ Perhaps add the following short-hands:
 * `struct.new <typeidx>` allocates a structure of type `$t` and initialises its fields with given values


rossberg · 2020-06-04T12:29:27Z

proposals/gc/MVP.md

    - iff `$t = struct (mut t)*`
-    - and `(type $t) <: t'`
+    - and `(type $t) == t'`


I'm confused by the (type $t) part. Isn't $t a type?

It's a "type use" referring to $t, see the new definition of heap types. The extra keyword avoids ambiguity in some syntactic contexts and is more consistent with other syntax. Note that (ref $t) is defined as a shorthand for (ref (type $t)).

FWIW, I just removed the type annotation on the instruction, because with type equivalence it can be derived from the operand type.

If I get to make further wishes: I find the t* part confusing too, because that's not a repeated sequence of a single t type.

It's a sequence of possibly different value types. It's a notational convention that is used heavily in the spec, explained here.

rossberg · 2020-06-04T12:30:41Z

proposals/gc/MVP.md

-* `rtt.canon <constype>` returns the RTT of the specified type
-  - `rtt.canon t : [] -> [(rtt t)]`
+* `rtt.canon <heaptype>` returns the RTT of the specified type
+  - `rtt.canon t : [] -> [(rtt 0 t)]`
  - multiple invocations of this instruction yield the same observable RTTs
  - this is a *constant instruction*
  - equivalent to `(rtt.sub t (rtt.canon any))`, except when `t` itself is `any`


Ah yes, fixed (it's (rtt.sub 1 any t (rtt.canon any))).

rossberg · 2020-06-04T12:31:49Z

proposals/gc/MVP.md

  - multiple invocations of this instruction with the same operand yield the same observable RTTs
  - this is a *constant instruction*

+TODO: Add the ability to generate new (non-canonical) RTT values to implement casting in nominal type hierarchies.


Not yet, it may be unnecessary in the presence of nominal types.

This is copied and modified from this closed PR: WebAssembly/bulk-memory-operations#19

State principal types

Clarifications and refinements

3a3b114

taralx reviewed May 30, 2020

View reviewed changes

binji requested a review from jakobkummerow June 1, 2020 16:05

Fixes

f7a5642

taralx reviewed Jun 2, 2020

View reviewed changes

Fix rtt depth

4b3fe05

jakobkummerow approved these changes Jun 3, 2020

View reviewed changes

Jakob's comments

4253221

rossberg commented Jun 4, 2020

View reviewed changes

Reinsert immediate

04e50f4

rossberg merged commit 20a80e3 into master Jun 5, 2020

rossberg deleted the clarif branch June 5, 2020 12:22

ajklein mentioned this pull request Aug 26, 2020

How much should GC be influenced by JS engine design? #125

Closed

rossberg pushed a commit that referenced this pull request Feb 24, 2021

[spec] Runtime format of element and data segments (#91)

a976b82

This is copied and modified from this closed PR: WebAssembly/bulk-memory-operations#19

rossberg added a commit that referenced this pull request Mar 22, 2023

Merge pull request #91 from WebAssembly/principal

314a335

State principal types

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More clarifications and tweaks #91

More clarifications and tweaks #91

rossberg commented May 28, 2020

rossberg commented May 28, 2020

binji commented May 28, 2020

taralx May 30, 2020

taralx commented May 30, 2020

rossberg commented Jun 2, 2020

taralx Jun 2, 2020

rossberg Jun 3, 2020

jakobkummerow Jun 3, 2020

rossberg Jun 4, 2020

jakobkummerow left a comment

jakobkummerow Jun 3, 2020

jakobkummerow Jun 3, 2020

rossberg Jun 4, 2020

jakobkummerow Jun 3, 2020

rossberg Jun 4, 2020

jakobkummerow Jun 4, 2020

rossberg Jun 5, 2020

jakobkummerow Jun 3, 2020

jakobkummerow Jun 3, 2020

rossberg Jun 4, 2020

rossberg left a comment

rossberg Jun 4, 2020

rossberg Jun 4, 2020

rossberg Jun 4, 2020

rossberg Jun 4, 2020

		@@ -186,34 +203,32 @@ Perhaps add the following short-hands:
		* `struct.new <typeidx>` allocates a structure of type `$t` and initialises its fields with given values

More clarifications and tweaks #91

More clarifications and tweaks #91

Conversation

rossberg commented May 28, 2020

rossberg commented May 28, 2020

binji commented May 28, 2020

Choose a reason for hiding this comment

taralx commented May 30, 2020

rossberg commented Jun 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobkummerow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment