lang: Split FuncValue into FuncValue and SimpleFn

[gelisam]

Representing an MCL function value as a golang function from Value to Value was a mistake, it should be a function from Vertex to Vertex.

Why this is a mistake

The output of a function like

$f = fn(x) {
  Shell(Sprintf("seq %d", x))
}

varies over time, while a single Value does not. Thus, code like

Map($f, list(1, 2))

would first produce the value list("1", "1"), but then it would not update to list("1", "2") when "seq 2" produces its second line. That's because with the mistaken design, when Map receives a new FuncValue or a new input list of N elements, Map calls the function from Value to Value N times and produces a single output list of N elements.

Why the corrected design is better

Here's what happens with this new design when Map receives a new FuncValue or a new input list of N elements.

First, Map constructs N item-input nodes, each of which extracts a different entry from the list. Then, Map calls the function from Vertex to Vertex N times, once for each item-input node, and thus obtain N item-output nodes. Finally, Map constructs an item-collecting node which constructs a list out of all of its inputs, and Map connects the N item-output nodes to the item-collecting node. This item-collecting node is the output of Map.

The Vertex to Vertex function constructs and connects its own nodes; in this case, it constructs an Sprintf node and connects the item-input node to it, and then constructs a Shell node and connects the Sprintf node to it, and then returns the Shell node as the item-output node.

The two Shell node in this sub-graph emit a first value "1", which propagates to the item-collecting node and causes it to output a first value list("1", "1"). Then, the second Shell node emits a second value "2", which propagates to the item-collecting node and causes it to output a second value list("1", "2"), as desired.

How this commit brings us closer to the above plan

Changing FuncValue throughout the codebase is a big change. One of the difficulties is that it is not just nodes which are emitting FuncValues, there are also many other places in the code where FuncValue is used to hold a golang function from Value to Value. Some of those places now need to hold a golang function from Vertex to Vertex, but other places still need to hold a golang function from Vertex to Vertex.

Thus, as a first step, we need to split FuncValue into two types. This commit splits the old FuncValue into two types:

1. The new FuncValue will hold a function from Vertex to Vertex. FuncValue is a Value.
2. A new type named "SimpleFn" will hold a function from Value to Value. SimpleFn is not a Value.

This commit replaces occurrences of the old FuncValue with one of those two new types, as appropriate. This commit does not yet adapt the surrounding code to make use of the new representation; that will be done in future commits. I have annotated the missing parts with the following panic message in order to make it easy to find which parts still need to be implemented. The "..." part explains what needs to be implemented.

panic("TODO [SimpleFn]: ...");

Where I need help

One part of the code which is not clear to me are the parts which use reflection. I don't understand the purpose of that code well enough to explain what needs to be implemented. I have annotated those "known unknown" parts of the remaining work with the following panic message in order to make it easy to find which parts still need more thinking and planning:

panic("TODO [SimpleFn] [Reflect]: ...");

[gelisam]

Since I have added a ton of panic calls, I do expect the tests to fail on CI, but I did not expect to see a type error! The error is in a file named lang/funcs/core/generated_funcs.go which I do not have on my machine, so I assume there's some code-generation going on somewhere which now needs to use SimpleFn instead of FuncValue in the generated code?

[gelisam]

Fixed generated_funcs.go.

[gelisam]

Reflowed comments.

[gelisam]

The static checks are now passing in CI! Of course, CI is now failing because the tests are failing, understandably so because of the panic calls, but that's intended.

[gelisam]

This PR has now been incorporated into #704. Closing.