add functions to the language #704

gelisam · 2023-03-09T04:30:04Z

This PR is not yet complete. It exists to make it easy to look at and comment on the code for the mcl-functions feature branch, which completely reworks the way in which functions work in MCL.

The problem

Today in master, builtins and lambdas use very different compilation strategies. A builtin is compiled to a single Func which receives values from upstream nodes and emits values downstream. A lambda, by contrast, is compiled away, as the lambda's body is inlined at every call site. Once this inlining is performed, only builtins remain, so the resulting function graph is the graph of their Funcs.

This does not work at all for MapFunc, a builtin which expects to receive a FuncValue as one of its inputs, because the above compilation strategy does not produce FuncValues, neither for builtins nor for lambdas. It would in fact be incorrect to construct a FuncValue from a builtin, because builtins can have an internal state and emit multiple values over time (e.g. System("seq 3") emits "1", then "2", then "3"), but a FuncValue can only transform input values into a single output value, so it cannot represent that complexity. And of course, lambdas can contain builtins (e.g. func(x) { System(x) }), so they cannot be represented by a FuncValue either.

The solution

The function graph must be able to change over time, because if func(x) { Map(System, xs) } first receives a list of length 1 and then a list of length 2, then at first there must be one SystemFunc in the function graph, then later there must be 2 SystemFuncs.

In order to do this, a FuncValue must be able to do more than transform input values into a single output value. Instead a FuncValue must describe in which way the function graph needs to change in order to accommodate a new instance of the function which the FuncValue represents. This branch's FuncValue does this by receiving input Funcs, creating and adding new nodes to the function graph, and then returning the one node among those new creations which shall be treated as the output Func, that is, the one whose output values are the result of applying the function represented by the FuncValue to the inputs received from the input nodes.

Another big change is that the function engine now works on a graph of Funcs instead of a graph of Exprs. This allows FuncValues to add Funcs to the function graph, without having to wrap them in Expr wrappers which don't correspond to any MCL code the user wrote.

Overview of the changes so far

FuncValue now behaves as described above.
A new type called SimpleFn behaves in the same way the old FuncValue did, because it is still useful in many places to represent a transformation from input values to a single output value.
Txn, an abstract API for modifying a function graph transactionally.
graphTxn, a thread-safe implementation of that API for modifying the function graph while the function engine is still running that graph.
ReversibleTxn, a wrapper around Txn which can later remove the nodes and edges which were added. This is useful because if xs becomes a list of length 1 again, we must remove the now-disconnected SystemFunc node we created for the length 2 case.
GraphTxn, a single-threaded implementation of that API which constructs a graph. Used to translate between the parts of the code which add nodes using this API and the parts which return a new Graph value.
Reversible.AddGraph(), to translate in the opposite direction.
Reimplemented CallFunc.Stream() under the new paradigm
Combine Expr.Graph() and Expr.Func() into a single method, Expr.MergedGraph(). Since the function graph no longer receives a graph of Exprs, it cannot call Func() on those Exprs in order to obtain the Funcs it needs, instead the graph returned by Expr.Graph() must already be a graph of Funcs.
MergedGraph() returns the Func in the graph whose output values correspond to the values emitted by the Expr. The caller can no longer assume that this node is the Expr itself, as the graph no longer contains Exprs.
MergedGraph() receives an environment mapping variable names to Funcs. ExprVars can no longer lookup the Expr which defines the variable in the Scope, because they now need to connect to a Func, not an Expr.
Implemented ExprCall.MergedGraph()
Implemented ExprFunc.MergedGraph()
Implemented ExprVar.MergedGraph()
Removed FunctionFunc, as it is no longer used anywhere.
Utility functions for converting between SimpleFn, FuncValue, and Funcs.
Reimplement FuncValue.Call() under the new paradigm
Reimplement MapFunc.Stream() under the new paradigm
fill in MergedGraph()'s initial environment so it contains all the in-scope variables.
Detect when a CallFunc or MapFunc's subgraph no longer emits any new values and the subgraph can no longer change (because the upstream node is no longer sending new FuncValues), so that the CallFunc can also indicate that it won't be sending new values downstream.
Change the function engine to work with Funcs, not Exprs.
Change all the MergedGraph() implementation so they add Funcs to the graph, not Exprs.
Finish GraphTxn
Plumb the table through Output()
Add fancy testing infrastucture to validate that the new Lock/Unlock works in the function engine @purpleidea
Copy top-level definitions when then are used by an ExprVar or an ExprCall.
When Func. Stream() closes it's output channel, close the input channel of the downstream nodes, so that the tests finish instead of hanging. @purpleidea
Use reference counting, so that transaction 0 can add vertex A, then transaction 1 also adds vertex A, then whether it is transaction 0 or 1 which reverses next, vertex A is not yet deleted, then when the other transaction reverses, vertex A is deleted. @purpleidea (Needs tests though!)
@purpleidea to get rid of the interpret_test.go:1202: test #62: funcs: process end err: context canceled... messages. Those are red herrings since we're interrupting process and it's normal.
Use a transaction to add the initial graph, and then reverse it at the end of the test, and assert that the graph is now empty. @purpleidea interpret_test.go:1546
Allow a Func to have two edges pointing to the same Func? Or something equivalent? @purpleidea Note: the two edges are always going to be added (and later reversed) in the same transaction.
improve the type unification solver so that the identity function applied to a string is accepted @purpleidea
@gelisam make classes work again
@purpleidea A panic inside of dage/process/etc should not HANG the engine. We should still unblock and exit cleanly. (I think this is fixed, but we don't have tests for it.)

Remaining work

@gelisam To add some more tests for map and other things he thinks we should be catching.
@purpleidea to make a txtar style test harness to model map function changing its inputs over time, or other things as well.
Add input caching to prevent unnecessary churn to map. Decide on if we should do this or not. Probably yes.
See if we can wrap the code in map to generalize it for writing new functions like reduce, filter, and any other lambda-taking funcs.
@purpleidea should rewrite a lot of lib/main.go and and update the GAPI api as well.
@gelisam How hard is it to pass a function to a resource (we just pass int/string/list/etc now) and what are the implications of this if any? Eg: Do they need to be entirely pure? Can they have side effects? Etc...
@gelisam See how a compiler optimization would work so that non-lambda functions (normal ones) appear as simple static additions to the graph so that they don't require to Txn Commit/Reverse a bunch of stuff and unnecessarily hurt graph performance.

Given the number of changes, I would be surprised if all of that worked the first time, so in addition to the above, I expect a long debugging session to make all the tests pass.

purpleidea · 2023-03-09T17:43:31Z

Sweeet =D

purpleidea · 2023-04-07T19:43:23Z

FYI: I updated merged graph branch. You may wish to cherry-pick stuff into here. I can fix the merge conflict if necessary.

https://github.com/purpleidea/mgmt/tree/feat/merged-graph

Of note: I added caching of the function pointer returned from Func(). This way, if it's called more than once, the same func will be returned. Is this a problem for us? Of course if we Copy() the Func Expr, then the caching doesn't get copied...

If this approach is satisfactory, then this allows us to look at the functions from the function graph, and determine which ones maps to which by calling Func again everywhere.

This isn't as effective performance wise as adding a new map to our returned function signature, but at least for now, I think this is easier to plumb in, and once we're sure we don't need to change the signature any more, we can revisit in the future.

purpleidea · 2023-05-12T22:25:25Z

FYI: debugging test 42:

I set:

runGraphviz || true

to enable the function graph output.

Then I ran:

time go test github.com/purpleidea/mgmt/lang/ -run TestAstFunc2/test_#42

Which gets us the error:

    interpret_test.go:1605: test #42: stream errored: func `printf@0xc0006383f0` stopped before it was loaded

Which also produced /tmp/graphviz.dot.png

Not the two constants are not attached to the function. So it never gets those inputs. Which is why we see the error.

The correct output looks roughly like this (from git master)

gelisam · 2023-05-13T01:06:27Z

The graph in which the two const nodes are not attached to anything looks correct to me. What is supposed to happen is that the printf node sends a FuncValue to the call node, which then creates a sub-graph which looks like the V-shaped graph from master, plus a ChannelBasedSink which sends the output of printf back to the call node via a channel. Is it possible that the test harness stop with the error "stream errored: func ***@***.***` stopped before it was loaded" before the call node is given a chance to generate it's sub-graph? Another thing I notice about this example is that there are two very different printf nodes in this example, the one with in-degree zero which produces a FuncValue, and the one with in-degree 2 in the V-shaped sub-graph. So another possible explanation for the bug might be that we're using the wrong one in one of the two graphs.

…

On Fri, May 12, 2023, 18:25 James ***@***.***> wrote: FYI: debugging test 42: I set: runGraphviz || true to enable the function graph output. Then I ran: time go test github.com/purpleidea/mgmt/lang/ -run TestAstFunc2/test_#42 Which gets us the error: interpret_test.go:1605: test #42: stream errored: func ***@***.***` stopped before it was loaded Which also produced /tmp/graphviz.dot.png [image: graphviz dot] <https://user-images.githubusercontent.com/135091/238090580-ece5d436-53b8-4b70-b55d-41c92ec2b0de.png> Not the two constants are not attached to the function. So it never gets those inputs. Which is why we see the error. The correct output looks roughly like this (from git master) [image: graphviz dot] <https://user-images.githubusercontent.com/135091/238090954-d45700fb-3838-4adf-a623-6864544392e5.png> — Reply to this email directly, view it on GitHub <#704 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAL62D6LFKMFDZA2VTT2CLXF22GBANCNFSM6AAAAAAVUSX4KA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

purpleidea · 2023-05-13T02:14:11Z

Sorry, I forgot to include the mcl code:

test fmt.printf("sq2: %d", 42)

Another thing I notice about this example is that there are two very different printf nodes in this example

Good point, I should have covered that sorry. Unfortunately, I don't think there are. The reason the nodes look slightly different is that in the earlier version, we're running a .String() to get the label on the Expr (which is what is in the function graph) where as in the newer version, we've had to simplify the label tag because we don't have as much context as the actual node is based on the function implementation (no more expr!). But the actual Stream() implementations in printf doing the work are identical.

Is it possible that the test harness stop with the error "stream errored: func @.***` stopped before it was loaded" before the call node is given a chance to generate it's sub-graph?

I don't believe so.

The reason we get this error is that the singular printf node which is running in the function engine receives no input (IOW, no incoming edges) so the engine closes the entire input stream. The printf function, seeing it is not getting any more inputs, then shuts down the output stream. Finally, the function engine notices (thanks engine!) that an output stream has shutdown without ever once producing a value, and assumes (rightly so) that there is a bug. All nodes are expected to either produce at least one value or error.

The current printf implementation indeed is supposed to receive one or more values (format string and optional args) in the function engine. If this is now supposed to go through a channelsink or channelsource, then the fundamental API of every static arg list function (basically everything except map/reduce/etc) would need to change. I don't think that's what we were trying to accomplish. I do believe we probably just forgot two edges in this case.

gelisam · 2023-05-13T03:34:02Z

the actual Stream() implementations in printf doing the work are identical.

Then that's the bug. The printf node with in-degree zero (let's call it printf-zero) should not be doing the same work as the printf node which in-degree two (let's call it printf-two). printf-zero should produce a FuncValue which creates a sub-graph containing printf-two, while printf-two should produce a string based on its arguments.

the function engine notices (thanks engine!) that an output stream has shutdown without ever once producing a value, and assumes (rightly so) that there is a bug.

That makes sense. The bug is that we give zero incoming edges to printf-two, which behaves like you explained when it has no incoming edges are missing. We should instead give zero incoming edges to printf-zero, which does produce a value when it has no incoming edges.

If this is now supposed to go through a channelsink or channelsource, then the fundamental API of every static arg list function (basically everything except map/reduce/etc) would need to change. I don't think that's what we were trying to accomplish.

We do want to change the semantics of every single MCL function, that's the premise of this PR: previously, every single MCL function was converting values to values, that doesn't work, so we now want every single MCL function to be generating sub-graphs instead.

Luckily, that doesn't mean that we have to change hundreds of function implementations throughout the codebase. We still need those Func implementations, we just need to make sure that the node which feeds the call node is not one of those Func implementations, and is instead a wrapper which returns a FuncValue which creates a sub-graph which contains one of those Func implementations.

gelisam · 2023-05-13T04:31:25Z

I have pushed an attempt at fixing the problem, but it doesn't work. The behaviour for our simplified test 42 is now non-deterministic :(

Sometimes it fails with stream errored: func 'subgraphOutput' stopped before it was loaded, as before but with subgraphOutput instead of printf.

Sometimes it fails with that message and then hangs.

Sometimes if fails with field arg1 XXX does not exist. That one makes sense, it's the same bug we encountered and fixed together during our last pairing session: I was not careful to use the correct edge names, because I didn't realize that the function graph used named parameters rather than positional parameters. In both places where I use the name arg%d XXX, there is a Type value nearby, that type should be a function type, and we should be able to extract the correct argument names from it.

purpleidea · 2023-05-13T04:40:13Z

The behaviour for our simplified test 42 is now non-deterministic :(

It's possible there is a race condition in either my engine or the subgraph channel stuff.

Function graph generation should be deterministic though.

Adding this line in TestAstFunc2:

runGraphviz || true

Then running the test

Will produce /tmp/graphviz.dot.png

if graphviz is installed, and can at least confirm the correct function graph shapes.

If we're satisfied with that shape, we can look into seeing where in the engine and/or call funcs the races are.

purpleidea · 2023-05-13T04:42:15Z

Btw:

I didn't realize that the function graph used named parameters rather than positional parameters

Indeed, since params come in as edges, there's no obvious ordering-- the only solution is to have them be named.

In both places where I use the name arg%d XXX, there is a Type value nearby

What do you mean by nearby?

gelisam · 2023-05-13T04:51:42Z

What do you mean by nearby?

The T field of the enclosing FuncValue definition.

gelisam · 2023-05-13T13:13:48Z

Adding this line in TestAstFunc2

Where? TestAstFunc2 is a folder, not a file.

gelisam · 2023-05-13T13:14:31Z

ah, it's also a function in ./lang/interpret_test.go

gelisam · 2023-05-13T13:22:34Z

If we're satisfied with that shape, we can look into seeing where in the engine and/or call funcs the races are.

(I see the same graph regardless of which of the non-deterministic failures occurs)

That looks correct. The const going into call is the ConstFunc which returns a FuncValue and closes the stream, and the other two const nodes are the ConstFunc are the ConstFuncs for "sq2: %d" and 42. Is there a way to see the function graph after the sub-graph is created?

gelisam · 2023-05-13T17:12:19Z

Oh! I bet the arg%d XXX is the root cause. When printf-two receives two mis-labelled inputs, it errors out, closing its output stream before emitting any value. The non-determinism is merely about which of the two panics is displayed first. Not sure about the hang though...

purpleidea · 2023-05-15T18:45:12Z

Oh! I bet the arg%d XXX is the root cause. When printf-two receives two mis-labelled inputs...

If you instead run:

test fmt.printf("onlyonearg")

Does it still display that non-determinism?

purpleidea · 2023-05-15T22:23:45Z

I decided I wanted to double check I hadn't made a mistake somewhere in the deps for all of this (engine, txn, etc) so I wrote a test. Indeed I found a small bug (woops) and while I don't think it necessarily caused this issue, I figured I'd mention it in case.

The bug and associated test is in the txn code. I rebased it and feat/merge-stuff contains the fix.
I also rebased a copy of this branch (which is rebased on top of that merge-stuff branch) to save you doing the work if you'd like. That branch is feat/sam-squashed-rebased.

purpleidea · 2023-05-15T22:31:08Z

I tried this branch of yours (with the txn fix, and just one arg, the format string, for the printf) and I get:

    interpret_test.go:1234: test #42: funcs: Running func `subgraphOutput`
    interpret_test.go:1234: test #42: funcs: not yet loaded: call
    interpret_test.go:1234: test #42: funcs: Running func `printf@0xc000279980`
    interpret_test.go:1604: test #42: FAIL
    interpret_test.go:1605: test #42: stream errored: func `subgraphOutput` stopped before it was loaded
    interpret_test.go:1234: test #42: funcs: Exiting func `printf@0xc000279980`
    interpret_test.go:1234: test #42: funcs: Exiting func `subgraphOutput`

Meaning I think there might be a channel bug in the func code.

I also notice that we hang at:

obj.wg.Wait() // don't cleanup these before Run() finished

In the engine. This leads me to believe there is a channel bug in your function code. I can help debug that with you.

BUT to be 100% sure it's not my fault (it might be!) I'm working on a real test for the engine stuff so I can be sure it's not the engine's fault. (I don't think so, but it's the most productive thing I can think to do on my own.)

FAIL: TestAstFunc1/test_#16_(module_search1.txtar) FAIL: TestAstFunc1/test_#21_(recursive_module1.txtar)

gelisam · 2023-09-20T03:12:54Z

Do you know how to fix these up?

As a first step, let's use

panic(fmt.Sprintf("ExprRecur node %s still present after SetScope()", obj.Name))

instead of

panic("ExprRecur node still present after SetScope()")

so we can see which identifier is causing the problem. In module_search1.txtar, it's mod1.name. Eliminating the extraneous cases to hone in on the problematic code:

-- metadata.yaml --
main: "main/hello.mcl"  # this is not the default, the default is "main.mcl"
files: "files/"         # these are some extra files we can use (is the default)
path: "path/"           # where to look for modules, defaults to using a global
-- main/hello.mcl --
import "mod1/"

test $mod1.name {}
-- main/mod1/metadata.yaml --
# empty metadata file (use defaults)
-- main/mod1/main.mcl --
import "mod1/"  # the nested version, not us

$name = "this is module mod1 which contains: " + $mod1.name
-- main/mod1/mod1/metadata.yaml --
# empty metadata file (use defaults)
-- main/mod1/mod1/main.mcl --
$name = "this is the nested local module mod1"

We can see that in $name = "this is module mod1 which contains: " + $mod1.name is probably the line which is causing the issue. Changing $mod1.name to $mod1.nested_name (and similarly in main/mod1/mod1/main.mcl) fixes the problem, so the bug is not that $mod1.name accidentally refers to main/mod1/main.mcl's definition instead of main/mod1/mod1/main.mcl's. I think the bug is probably that ExprRecur overwrites main/mod1/mod1/main.mcl's reference instead of main/mod1/main.mcl's.

gelisam · 2023-09-20T03:42:55Z

Since $mod1.name is a variable reference, not a function call, the problem must be the definitionScope.Variables[obj.Name] = &ExprRecur{obj.Name} in ExprVar.SetScope().

Ah, I see the problem! This code was written to catch this case:

$name = $name + 1
test $name {}

The idea is that when we encounter the $name in test $name {}, we jump to the RHS of the $name = .... In order to prevent the recursion, we poison the binding for $name in .... This poisoning works in this case, but is incorrect in general.

In the problematic test, when we encounter test $mod1.name {}, we jump to the RHS of $name = "this is module mod1 which contains: " + $mod1.name in main/mod1/main.mcl. In order to prevent recursion, we poison the binding for $mod1.name in "this is module mod1 which contains: " + $mod1.name, but this time this poisons the wrong binding.

The simple solution is simply to get rid of ExprRecur; you already detect recursion via other means anyway.

it isn't always correct, and recursion is already detected via other means anyway

gelisam · 2023-09-20T03:48:22Z

I have reset to your branch and dropped ExprRecur. obviously, the two tests no longer fail with the panic call I deleted. the tests still fail though.

purpleidea · 2023-09-20T12:35:17Z

I have reset to your branch and dropped ExprRecur. obviously, the two tests no longer fail with the panic call I deleted. the tests still fail though.

Not an issue, that's just the expected graph output format being different-- I fixed up all the others, but of course I couldn't fix this one until I knew what the output was supposed to be-- we're basically testing for stability at this point, instead of being precise and comparing to expected.

Anyways, great news! These all pass too now, haha!

It's sort of hilarious because weren't we just talking about ExprRecur being weird the other day? Amazing work!

purpleidea · 2023-09-20T13:13:53Z

It seems a deadlock has crept in, or maybe it was just never seen previously :/ Hmmm.

I think it happens with subtest 47

time go test github.com/purpleidea/mgmt/lang/ -run TestAstFunc2/test_#47


panic: test timed out after 10m0s

goroutine 3729 [running]:
testing.(*M).startAlarm.func1()
	/usr/lib/golang/src/testing/testing.go:2036 +0x8e
created by time.goFunc
	/usr/lib/golang/src/time/sleep.go:176 +0x32

goroutine 1 [chan receive, 10 minutes]:
testing.(*T).Run(0xc000103ba0, {0x1bfa4e1?, 0x54f725?}, 0x1c8d278)
	/usr/lib/golang/src/testing/testing.go:1494 +0x37a
testing.runTests.func1(0xc000103ba0?)
	/usr/lib/golang/src/testing/testing.go:1846 +0x6e
testing.tRunner(0xc000103ba0, 0xc0007ffcd8)
	/usr/lib/golang/src/testing/testing.go:1446 +0x10b
testing.runTests(0xc0004272c0?, {0x2cc92c0, 0xa, 0xa}, {0x7f26b40e95b8?, 0x40?, 0x2d3c260?})
	/usr/lib/golang/src/testing/testing.go:1844 +0x456
testing.(*M).Run(0xc0004272c0)
	/usr/lib/golang/src/testing/testing.go:1726 +0x5d9
main.main()
	_testmain.go:65 +0x1aa

goroutine 23 [chan receive, 9 minutes]:
testing.(*T).Run(0xc000103d40, {0xc000488c60?, 0xc000093f40?}, 0xc000833680)
	/usr/lib/golang/src/testing/testing.go:1494 +0x37a
github.com/purpleidea/mgmt/lang.TestAstFunc2(0xc000103d40)
	/home/james/code/mgmt/lang/interpret_test.go:599 +0x985
testing.tRunner(0xc000103d40, 0x1c8d278)
	/usr/lib/golang/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
	/usr/lib/golang/src/testing/testing.go:1493 +0x35f

goroutine 3791 [semacquire, 9 minutes]:
sync.runtime_Semacquire(0xc00051d588?)
	/usr/lib/golang/src/runtime/sema.go:62 +0x25
sync.(*WaitGroup).Wait(0xc00007e2d0?)
	/usr/lib/golang/src/sync/waitgroup.go:139 +0x52
github.com/purpleidea/mgmt/lang.TestAstFunc2.func1(0xc000353520)
	/home/james/code/mgmt/lang/interpret_test.go:1231 +0x5abb
testing.tRunner(0xc000353520, 0xc000833680)
	/usr/lib/golang/src/testing/testing.go:1446 +0x10b
created by testing.(*T).Run
	/usr/lib/golang/src/testing/testing.go:1493 +0x35f

goroutine 3690 [chan receive, 9 minutes]:
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run.func4()
	/home/james/code/mgmt/lang/funcs/dage/dage.go:885 +0x10e
created by github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run
	/home/james/code/mgmt/lang/funcs/dage/dage.go:877 +0x4c5

goroutine 3689 [semacquire, 9 minutes]:
sync.runtime_Semacquire(0x0?)
	/usr/lib/golang/src/runtime/sema.go:62 +0x25
sync.(*WaitGroup).Wait(0x18b1580?)
	/usr/lib/golang/src/sync/waitgroup.go:139 +0x52
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).deleteVertex.func1()
	/home/james/code/mgmt/lang/funcs/dage/dage.go:396 +0x33
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).runNodeWaitFns(0xc0004cba20)
	/home/james/code/mgmt/lang/funcs/dage/dage.go:573 +0xb0
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run(0xc0004cba20, {0x1ff4758, 0xc000916b80})
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1084 +0xfde
github.com/purpleidea/mgmt/lang.TestAstFunc2.func1.6()
	/home/james/code/mgmt/lang/interpret_test.go:1029 +0x8a
created by github.com/purpleidea/mgmt/lang.TestAstFunc2.func1
	/home/james/code/mgmt/lang/interpret_test.go:1027 +0x407b

goroutine 3691 [semacquire, 9 minutes]:
sync.runtime_Semacquire(0xc0003bd680?)
	/usr/lib/golang/src/runtime/sema.go:62 +0x25
sync.(*WaitGroup).Wait(0xc000916c00?)
	/usr/lib/golang/src/sync/waitgroup.go:139 +0x52
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run.func5()
	/home/james/code/mgmt/lang/funcs/dage/dage.go:941 +0x96
created by github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run
	/home/james/code/mgmt/lang/funcs/dage/dage.go:934 +0x634

goroutine 3751 [chan receive, 9 minutes]:
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run.func9({0x1ff6bc0, 0xc000896c30}, 0xc000500460)
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1238 +0x13f
created by github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1224 +0x16f2

goroutine 3750 [chan send, 9 minutes]:
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Lock(0xc0004cba20)
	/home/james/code/mgmt/lang/funcs/dage/dage.go:510 +0x2d
github.com/purpleidea/mgmt/lang/funcs/dage.(*graphTxn).commit(0xc0005000e0)
	/home/james/code/mgmt/lang/funcs/dage/txn.go:448 +0x58
github.com/purpleidea/mgmt/lang/funcs/dage.(*graphTxn).Reverse(0xc0005000e0)
	/home/james/code/mgmt/lang/funcs/dage/txn.go:592 +0x28d
github.com/purpleidea/mgmt/lang/funcs/structs.(*CallFunc).Stream.func1()
	/home/james/code/mgmt/lang/funcs/structs/call.go:117 +0x2a
github.com/purpleidea/mgmt/lang/funcs/structs.(*CallFunc).Stream(0xc000896c30, {0x1ff4758, 0xc0007c2480})
	/home/james/code/mgmt/lang/funcs/structs/call.go:183 +0x5c2
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run.func8.1({0x1ff4758?, 0xc0007c2480?})
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1199 +0x93
github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run.func8({0x1ff6bc0?, 0xc000896c30?}, 0xc000500460)
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1201 +0x246
created by github.com/purpleidea/mgmt/lang/funcs/dage.(*Engine).Run
	/home/james/code/mgmt/lang/funcs/dage/dage.go:1179 +0x15d2
FAIL	github.com/purpleidea/mgmt/lang	600.143s
FAIL

purpleidea · 2023-09-20T13:20:15Z

Seems it's actually a bunch that can fail. Time to bisect everything =D

purpleidea · 2023-09-25T23:25:31Z

I'm proposing to merge all of this work in #719
More details there! \o/

purpleidea · 2023-09-25T23:34:06Z

Okay merged in #719 !

TristanCacqueray · 2023-09-26T14:50:24Z

Congrats on the feature and the new release!

This was referenced Mar 9, 2023

lang: Split FuncValue into FuncValue and SimpleFn #696

Closed

lang: CallExpr must generate a subgraph #697

Closed

lang: ExprFunc must produce a FuncValue #701

Closed

last few MergedGraph() implementations #703

Closed

gelisam force-pushed the mcl-functions branch 3 times, most recently from 16885bc to db9aaac Compare April 2, 2023 20:41

gelisam force-pushed the mcl-functions branch from b6c164a to c02e425 Compare April 9, 2023 18:25

gelisam force-pushed the mcl-functions branch 3 times, most recently from 856c0ba to a751e1d Compare May 7, 2023 21:07

gelisam closed this May 13, 2023

gelisam reopened this May 13, 2023

purpleidea added 15 commits September 19, 2023 14:46

XXX misc

5ab4724

XXX remove pdage for now

a0a2cef

XXX: misc cleanups

77f2842

XXX: fix old unification test, move it to new system

57c6530

XXX: misc tests port sort of

5926dfc

XXX: empty graphs are allowed too

edb74e6

XXX fix most tests except...

721504c

FAIL: TestAstFunc1/test_#16_(module_search1.txtar) FAIL: TestAstFunc1/test_#21_(recursive_module1.txtar)

XXX port more tests

2734547

XXX add missing refcount

61e7d29

XXX most tests needed this

6f99b1d

XXX fix last test

e071fdb

XXX txn tests

1c4ccc9

XXX more txn tests

f2f97d0

XXX when did i forget to have this here?

b09a5de

XXX: golint fixes

b058ed9

clearer ExprRecur errors

3434bc6

drop ExprRecur

25eb138

it isn't always correct, and recursion is already detected via other means anyway

gelisam force-pushed the mcl-functions branch from cf4214d to 25eb138 Compare September 20, 2023 03:47

purpleidea mentioned this pull request Sep 25, 2023

Feat/merge (PR#704 cleaned up) #719

Merged

purpleidea closed this Sep 25, 2023

gelisam mentioned this pull request Dec 4, 2023

Duplicate functions being created #722

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add functions to the language #704

add functions to the language #704

gelisam commented Mar 9, 2023 •

edited by purpleidea

Loading

purpleidea commented Mar 9, 2023

purpleidea commented Apr 7, 2023

purpleidea commented May 12, 2023

gelisam commented May 13, 2023 via email

purpleidea commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023 •

edited

Loading

purpleidea commented May 13, 2023

purpleidea commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

purpleidea commented May 15, 2023 •

edited

Loading

purpleidea commented May 15, 2023

purpleidea commented May 15, 2023 •

edited

Loading

gelisam commented Sep 20, 2023

gelisam commented Sep 20, 2023

gelisam commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 25, 2023

purpleidea commented Sep 25, 2023

TristanCacqueray commented Sep 26, 2023

add functions to the language #704

add functions to the language #704

Conversation

gelisam commented Mar 9, 2023 • edited by purpleidea Loading

The problem

The solution

Overview of the changes so far

Remaining work

purpleidea commented Mar 9, 2023

purpleidea commented Apr 7, 2023

purpleidea commented May 12, 2023

gelisam commented May 13, 2023 via email

purpleidea commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023 • edited Loading

purpleidea commented May 13, 2023

purpleidea commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

gelisam commented May 13, 2023

purpleidea commented May 15, 2023 • edited Loading

purpleidea commented May 15, 2023

purpleidea commented May 15, 2023 • edited Loading

gelisam commented Sep 20, 2023

gelisam commented Sep 20, 2023

gelisam commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 20, 2023

purpleidea commented Sep 25, 2023

purpleidea commented Sep 25, 2023

TristanCacqueray commented Sep 26, 2023

gelisam commented Mar 9, 2023 •

edited by purpleidea

Loading

gelisam commented May 13, 2023 •

edited

Loading

purpleidea commented May 15, 2023 •

edited

Loading

purpleidea commented May 15, 2023 •

edited

Loading