Add super basic generics - project static #376

BeksOmega · 2020-09-22T23:12:51Z

Preface

This is a bit of a weird PR because it's mostly puzzle pieces that will hopefully come together later. But I'll do my best to explain where my brain is going.

Also, I know this PR description is super long, and I still don't think I did a good job explaining. So if there's anything that's unclear or anything I can do to make it more clear please tell me.

Basic Idea

Let's start with a simple example of generics. Below there is a generic Identity block with a connection check T on its input and output. After we connect the Dog block to the Identity block we want the T check to start acting like a Dog check.

Now say we connect annother Identity block to the first Identity block. We now want that block's T check to /also/ start acting like a Dog check.

There are two ways we could implement this.

Whenever we connect a connection with a generic check to another connection, we create a "pointer" to the connection check of the other connection. Then when we go to get the explicit type of the generic check, we do a recursive lookup to see if it is bound to an explicit type anywhere.
So if we were to connect these two Identity blocks their T types would be bound to eachother.
We only "bind" a generic check to another check if the other check is an explicit type, or has an explicit type associated with it.
So if were were to connect these two Identity blocks their T types would /not/ be bound to eachother.

But if we added a Dog block to the second one, then our bindings would update to look like this:

I decide to go with option 2. The logic will be more complicated, but I believe it will be more efficient.

Efficiency

For example, say we implement option 1 and then I build the following set of blocks:

Each time I connect an Identity block, the growing blob of blocks would need to recursively check every connected block to see if it is bound to an explicit anywhere.

Whereas option 2 wouldn't require any of that, because none of the blocks are bound yet. It would only require a recursive update if/when I were to eventually connect an explicit type.

Complicated logic

To implement option 2 we need to do different things after two connections connect, depending on the "kinds" of the connections.

So currently a connection check can have three different "kinds"
A) It can be explicit, eg Dog - E
B) It can be generic, and not currently bound to an explicit type - GU
C) It can be generic, and bound to an explicit type eg Dog - GB

Note that we do want to bind already bound generics (GBs) to Es and other GBs. The reasoning for this is explained below in the PriorityQueueMap section.

Parent	Child	Action
E	E	Bind neither
E	GU	Bind child to parent
E	GB	Bind child to parent
GU	E	Bind parent to child
GU	GU	Bind neither
GU	GB	Bind parent to child
GB	E	Bind parent to child
GB	GU	Bind child to parent
GB	GB	Bind both to each other

This is what the logic here is meant to do.

We also need to do similar unbinding when the two blocks disconnect, except it's more complicated, because once an unbound connection is connected, it becomes bound.

Parent	Child	Action
E	E	Unbind Neither
E	GU	------
E	GB	Unbind child from parent
GU	E	------
GU	GU	Unbind neither
GU	GB	------
GB	E	Unbind parent from child
GB	GU	------
GB	GB	Unbind both to each other

Luckily the unbinding logic pretty much works even though connections that were previously GUs are now GBs (since they've been bound by connecting). We just have to make sure that the GenericMap (see below) fails graceful if we try to delete a binding and the binding doesn't exist.

Current logic limitiations

Note that currently the logic doesn't handle updating the checks that are dependent on a block when a block updates.

So if you connect the explicit block first, and then continue connecting generic blocks all of the bindings "flow through" the connections.

But if you connect the generic blocks together first, and then connect the explicit block, that's not handled yet.

Puzzle Pieces

PriorityQueueMap

This is a new data structure I created that works sort of like a map, except that keys are mapped to priority queues of values. So a key can be bound to multiple values, but it's actual value(s) is the value with the highest priority.

This new data structure is used to bind generic type names to their explicit type names, within the context of a block. It will probably also be used for generic type constraints, once I get there.

The reason we cannot implement type binding using a simple map is twofold:

We want outside devs to be able to override type bindings using fields or other UI they create (hence the priority).
We want to be able to bind generic types to multiple explicit types so that they can easily adapt to changing connections.

For example right now the generic type (T) of the Select Random block is unbound:

Now it should be bound to Dog:

Now Mammal:

And now Cat:

Allowing a type to have multiple bindings feels to me like the best way to handle any binding being removed at any time, and forcing the generic type to act like one of its other types.

Generic Map

The NominalConnectionChecker decides when to bind and unbind types, but the GenericMap handles the consequences of that decision (basically).

The generic map has two things it keeps track of.

For a given generic type on a given block, what explicit type is it currently bound to?
For a given generic type on a given block, what generic types on other blocks are dependent on the explicit type of this type?

(1) is useful for finding the explicit type of a generic type so that we can perform connection checks. (2) is useful for unbinding types. It will also allow us to have bindings “flow” through dependent types, once I add that.

Problems related to blockly APIs

So currently all of the binding is handled through event listeners, which causes two problems:

Generic checks don't act bound until we wait 1 ms for the event delay.
Generic checks don't get bound when loading from XML

Adding a way for a connection checker to get a callback immediately after connect & disconnect would solve both problems, but that's really only useful if you want to implement generics.

If we decide to stick with the events system I'll probably end up adding a Promises system to fix (1) and hooking into the FinishedLoading event for (2)

rachel-fenichel · 2020-09-25T00:08:05Z

plugins/nominal-connection-checker/src/priority_queue_map.js

+   * @param {*} value The value to unbind from the key.
+   * @param {number} priority The priority of the binding.
+   */
+  unbind(key, value, priority) {


Is it possible to have multiple values with the same priority?

It is possible to have:

Multiple different values with different priorities.

Multiple different values with the same priority.

Multiple of the same value with different priority.

Multiple of the same value with the same priority.

So basically: Yes. The PriorityQueueMap doesn't really have any fancy behavior atm, it's just a wrapper for a map where all of the values are priority queues.

rachel-fenichel · 2020-09-28T17:14:56Z

Okay, I'm diving back into this and will have a review for you today. Sorry about the delay!

plugins/nominal-connection-checker/src/index.js

rachel-fenichel

This makes sense overall, though it feels like the interactions are extremely complicated.

With respect to the deep generic nesting, I do wonder if you instead should make it so that you can't do Identity -> Identity -> Identity -> Dog. What if you blocked nested identity blocks entirely, so something has to be bound more immediately? (I have not thought all the way through this for all of the different use cases).

You may want to export a MIN_PRIORITY and MAX_PRIORITY for the generic map.

rachel-fenichel · 2020-09-28T19:45:41Z

plugins/nominal-connection-checker/src/index.js

+    if (e.newParentId) {
+      parentBlock = this.workspace_.getBlockById(e.newParentId);
+      parentCon = parentBlock.getInput(e.newInputName).connection;
+      explicitFn = GenericMap.prototype.bindTypeToExplicit.bind(genericMap);


Why are you using a bind instead of just calling the function, given that you also have the object around?

If the goal is only to make it possible to directly call the correct function, I think it would be clearer to set a boolean saying which case it is and then have a helper that checks that boolean and calls the appropriate function.

Yeah this is a really tricky spot, and I still have mixed feelings about it.

The reason I originally aliased these functions instead of just calling them is because I wanted to try and keep the logic at the end of the function as simple as possible. All it cares about is the state of the connections (E, GU, or GB) so I wanted to move the other info about binding vs unbinding somewhere else.

Adding checks for binding vs unbinding at the end of the function would definitely work, but my current feeling is that it would distract from the logic that's really important. I know it's annoying to refer back to the actual function that's getting called, but the idea is that you shouldn't have to do that often because again, that info isn't really important. What's important is when it calls the generic function vs when it calls the explicit function.

That's just my current feeling on it though. As this function expands (especially as I add logic for flowing bindings through connections) something else may be better.

rachel-fenichel · 2020-09-28T19:50:39Z

plugins/nominal-connection-checker/src/generic_map.js

+    if (!priorityMap) {
+      return undefined;
+    }
+    const types = priorityMap.getValues(genericType);


Is types guaranteed to have at least one entry if it's not null?

I checked, and the PriorityQueueMap handles that here. I'm not sure if I handled it correctly for the dependersMap_ though so I'll have to check. And I'll add some tests as well!

BeksOmega · 2020-09-28T22:39:41Z

Okay, I'm diving back into this and will have a review for you today. Sorry about the delay!

I started classes last week so this is actually a really good pace for me hehe.

This makes sense overall, though it feels like the interactions are extremely complicated.

They are quite complicated, which makes me a bit nervous lol. I'm worried that once I'm done, you and I are going to be the only ones that understand this system. Is there anywhere you think inline comments would be useful? Or somewhere I could write dev docs for it?

With respect to the deep generic nesting, I do wonder if you instead should make it so that you can't do Identity -> Identity -> Identity -> Dog. What if you blocked nested identity blocks entirely, so something has to be bound more immediately? (I have not thought all the way through this for all of the different use cases).

Hmmm are you saying that we should allow:

but disallow (ignoring the fact that this is allowed to connect to the Launch Flying Animal block when it should not be):

Or are you saying that we should make it impossible to chain identity blocks - no matter what order you connect them in?

You may want to export a MIN_PRIORITY and MAX_PRIORITY for the generic map.

Coolio, what do you think they should be? Right now all values from min int to max int are legal

rachel-fenichel · 2020-10-01T00:29:11Z

I was thinking that we you could make it impossible to chain identity checks regardless of the order.

I suggest 0 for a min priority. Int max for the max is fine.

As far as the complicated code goes--not sure, it's possible it'll be easier to understand when more of the pieces are in place.

rachel-fenichel · 2020-10-01T00:30:38Z

Approved--go ahead and merge when you're ready.

BeksOmega · 2020-10-01T02:06:40Z

I was thinking that we you could make it impossible to chain identity checks regardless of the order.

Ok cool! That's what I figured you were talking about, but I wanted to make sure.

I got really excited abou this idea (because it would simplfiy things /a lot/), so as a result I was thinking about it a lot. I think disallowing the connection of multiple generics could work well for simple generics like the ones I'm currently building, but I realized that it might run into problems once we get to parameterized types.

For example if you want to do array processing, you often only specify the type of the array when you define it. Here is some sudo-go that hopefully demonstrates.

// Takes in an age and returns a function that takes in an Animal and
// returns true if the Animal is greater than the given age.
func GreaterThanAge(age int) func(Animal) bool { }

// Takes in a weight and returns a function that takes in an Animal and
// returns true if the Animal is greater than the given weight.
func GreaterThanWeight(weight int) func(Animal) bool { }

// Takes in an array of type T and a function that takes in any type
// general-er than T that returns a boolean.
// This function returns an array of all of the elements that make the
// function return true. The type of the array is the original type.
func Filter(type T, U >: T)(array []T, fn func(U) bool) []T { }

// Creates an array of Dogs.
dogArray := []Dog{dog1, dog2, dog3}

// Removes all of the dogs that are less <= 5 years old and <= 10 pounds.
// As you can see the Dog type was only given once above, but filteredDogs
// should still be an array of type Dog.
filteredDogs := Filter(Filter(dogArray, GreaterThanAge(5)), GreaterThanWeight(10))

And here is that same thing but in sudo-blocks:

We want the Dog types to be able to flow through the generic List and Filter blocks.

So I think that if we /can/ get it to work we should. And I'm pretty sure we can get it there. Currently I've got the connecting all working, so I've (hopefully) just got to clean up some stuff with the disconnecting. We'll just have to wait and see!

BeksOmega · 2020-10-01T02:20:01Z

@moniika It looks like I might have messed something up with the field date tests... The results from travis look weird so here's what I get locally as well:

Console output

Ran at 19:09:17 PT

  1) FieldDate
       Constructor
         Empty:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:147:7)

  2) FieldDate
       Constructor
         Undefined:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  3) FieldDate
       Constructor
         Null:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  4) FieldDate
       Constructor
         NaN:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  5) FieldDate
       fromJson
         Empty:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:186:7)

  6) FieldDate
       fromJson
         Undefined:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  7) FieldDate
       fromJson
         Null:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  8) FieldDate
       fromJson
         NaN:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at assertFieldDefault (webpack-internal:///./test/field_date_test.mocha.js:64:5)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:98:7)

  9) FieldDate
       setValue
         Empty -> New Value
           Undefined:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

  10) FieldDate
       setValue
         Empty -> New Value
           Null:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

  11) FieldDate
       setValue
         Empty -> New Value
           NaN:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

  12) FieldDate
       setValue
         Empty -> New Value
           Non-Parsable String:

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

  13) FieldDate
       setValue
         Empty -> New Value
           Invalid Date - Month(2020-13-20):

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

  14) FieldDate
       setValue
         Empty -> New Value
           Invalid Date - Day(2020-02-32):

      Value
      + expected - actual

      -2020-09-30
      +2020-10-01
      
      at assertFieldValue (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:75:47)
      at Context.eval (webpack-internal:///./node_modules/@blockly/dev-tools/src/field_test_helpers.mocha.js:227:7)

I've got no clue what it is.

moniika · 2020-10-01T21:58:03Z

@moniika It looks like I might have messed something up with the field date tests... The results from travis look weird so here's what I get locally as well:

Console output
I've got no clue what it is.

I know what it is. It's on me. I made a mistake in the tests (with how I get the "expected" default value for the field. I'm going to make a PR to fix the test.

moniika · 2020-10-01T22:37:58Z

@moniika It looks like I might have messed something up with the field date tests... The results from travis look weird so here's what I get locally as well:
Console output
I've got no clue what it is.

I know what it is. It's on me. I made a mistake in the tests (with how I get the "expected" default value for the field. I'm going to make a PR to fix the test.

I think my PR should fix the Travis failures. The issues you have locally (with the date being off by 1 day) I thought were already fixed. Let me know if after merging if it works locally.

BeksOmega · 2020-10-01T23:00:56Z

I think my PR should fix the Travis failures. The issues you have locally (with the date being off by 1 day) I thought were already fixed. Let me know if after merging if it works locally.

Yep yep it works perfectly! I just forgot to rebase hehe. Thank you so much :D

rachel-fenichel reviewed Sep 25, 2020

View reviewed changes

rachel-fenichel reviewed Sep 28, 2020

View reviewed changes

plugins/nominal-connection-checker/src/index.js Outdated Show resolved Hide resolved

rachel-fenichel approved these changes Sep 28, 2020

View reviewed changes

rachel-fenichel approved these changes Oct 1, 2020

View reviewed changes

moniika mentioned this pull request Oct 1, 2020

Fix regex for creating expected date value for tests. #385

Merged

BeksOmega added 10 commits October 1, 2020 15:51

Add PriorityQueueMap

09584f0

Package updates

a9d2a19

Add other functionality

ad9b254

Add tests

f0630ec

Make generics caseless

97a6f1b

Cleanup

a5c04b4

Change PriorityQueueMap to use Map

c4eeb37

Remove TODO:

c52f12e

Add no values/bindings tests

7e71d7c

Add min & max priority

5905ec3

BeksOmega force-pushed the static/simple-generics branch from d2d7b39 to 5905ec3 Compare October 1, 2020 22:51

BeksOmega merged commit 09eb6d8 into google:master Oct 1, 2020

BeksOmega mentioned this pull request Oct 2, 2020

Nominal type checker w/ higher types & generics #346

Open

BeksOmega deleted the static/simple-generics branch April 5, 2022 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add super basic generics - project static #376

Add super basic generics - project static #376

BeksOmega commented Sep 22, 2020 •

edited

Loading

rachel-fenichel Sep 25, 2020

BeksOmega Sep 25, 2020

rachel-fenichel commented Sep 28, 2020

rachel-fenichel left a comment

rachel-fenichel Sep 28, 2020

BeksOmega Sep 28, 2020

rachel-fenichel Sep 28, 2020

BeksOmega Sep 28, 2020

BeksOmega commented Sep 28, 2020

rachel-fenichel commented Oct 1, 2020

rachel-fenichel commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

moniika commented Oct 1, 2020

moniika commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

Add super basic generics - project static #376

Add super basic generics - project static #376

Conversation

BeksOmega commented Sep 22, 2020 • edited Loading

Preface

Basic Idea

Efficiency

Complicated logic

Current logic limitiations

Puzzle Pieces

PriorityQueueMap

Generic Map

Problems related to blockly APIs

rachel-fenichel Sep 25, 2020

Choose a reason for hiding this comment

BeksOmega Sep 25, 2020

Choose a reason for hiding this comment

rachel-fenichel commented Sep 28, 2020

rachel-fenichel left a comment

Choose a reason for hiding this comment

rachel-fenichel Sep 28, 2020

Choose a reason for hiding this comment

BeksOmega Sep 28, 2020

Choose a reason for hiding this comment

rachel-fenichel Sep 28, 2020

Choose a reason for hiding this comment

BeksOmega Sep 28, 2020

Choose a reason for hiding this comment

BeksOmega commented Sep 28, 2020

rachel-fenichel commented Oct 1, 2020

rachel-fenichel commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

moniika commented Oct 1, 2020

moniika commented Oct 1, 2020

BeksOmega commented Oct 1, 2020

BeksOmega commented Sep 22, 2020 •

edited

Loading