lang: ast: structs, funcs: structs: Exprif without a channel

This adds an improved "expr if" which only adds the active branch to the
graph and removes the "secret" channel.

lang/ast/structs.go

@@ -12716,59 +12716,75 @@ func (obj *ExprIf) Check(typ *types.Type) ([]*interfaces.UnificationInvariant, e
 	return interfaces.GenericCheck(obj, typ)
 }
 
-// Func returns a function which returns the correct branch based on the ever
-// changing conditional boolean input.
-func (obj *ExprIf) Func() (interfaces.Func, error) {
-	typ, err := obj.Type()
-	if err != nil {
-		return nil, err
-	}
-
-	return &structs.IfFunc{
-		Textarea: obj.Textarea,
-
-		Type: typ, // this is the output type of the expression
-	}, nil
-}
-
 // Graph returns the reactive function graph which is expressed by this node. It
-// includes any vertices produced by this node, and the appropriate edges to any
-// vertices that are produced by its children. Nodes which fulfill the Expr
-// interface directly produce vertices (and possible children) where as nodes
-// that fulfill the Stmt interface do not produces vertices, where as their
-// children might. This particular if expression doesn't do anything clever here
-// other than adding in both branches of the graph. Since we're functional, this
-// shouldn't have any ill effects.
-// XXX: is this completely true if we're running technically impure, but safe
-// built-in functions on both branches? Can we turn off half of this?
+// includes the condition produced by this node, and the appropriate edges of
+// that. The then or else side of the graph is added at runtime based on the
+// value of the condition.
+// TODO: If we know the condition is static, generate only that side statically.
 func (obj *ExprIf) Graph(env *interfaces.Env) (*pgraph.Graph, interfaces.Func, error) {
 	graph, err := pgraph.NewGraph("if")
 	if err != nil {
 		return nil, nil, err
 	}
-	function, err := obj.Func()
+
+	typ, err := obj.Type()
 	if err != nil {
 		return nil, nil, err
 	}
 
-	exprs := map[string]interfaces.Expr{
-		"c": obj.Condition,
-		"a": obj.ThenBranch,
-		"b": obj.ElseBranch,
-	}
-	for _, argName := range []string{"c", "a", "b"} { // deterministic order
-		x := exprs[argName]
-		g, f, err := x.Graph(env)
+	// XXX: can we speculate if it's static?
+
+	g, f, err := obj.Condition.Graph(env)
 	if err != nil {
 		return nil, nil, err
 	}
 	graph.AddGraph(g)
 
-		edge := &interfaces.FuncEdge{Args: []string{argName}}
-		graph.AddEdge(f, function, edge) // branch -> if
+	thenGraph, thenFunc, err := obj.ThenBranch.Graph(env)
+	if err != nil {
+		return nil, nil, err
+	}
+	elseGraph, elseFunc, err := obj.ElseBranch.Graph(env)
+	if err != nil {
+		return nil, nil, err
 	}
 
-	return graph, function, nil
+	edgeName := structs.ExprIfFuncArgNameCondition
+	edgeNameDummy := structs.OutputFuncDummyArgName
+
+	exprIfSubgraphOutput := &structs.OutputFunc{ // the new graph shape thing!
+		Textarea: obj.Textarea,
+		Name:     "exprIfSubgraphOutput",
+		Type:     obj.typ,
+		EdgeName: structs.OutputFuncArgName,
+	}
+	graph.AddVertex(exprIfSubgraphOutput)
+
+	function := &structs.ExprIfFunc{
+		Textarea: obj.Textarea,
+
+		Type: typ, // this is the output type of the expression
+
+		EdgeName: edgeName,
+
+		ThenGraph: thenGraph,
+		ElseGraph: elseGraph,
+
+		ThenFunc: thenFunc,
+		ElseFunc: elseFunc,
+
+		OutputVertex: exprIfSubgraphOutput,
+	}
+	graph.AddVertex(function)
+
+	edge := &interfaces.FuncEdge{Args: []string{edgeName}}
+	graph.AddEdge(f, function, edge) // condition -> exprif
+
+	graph.AddEdge(function, exprIfSubgraphOutput, &interfaces.FuncEdge{
+		Args: []string{edgeNameDummy},
+	})
+
+	return graph, exprIfSubgraphOutput, nil
 }
 
 // SetValue here is a no-op, because algorithmically when this is called from

lang/funcs/structs/exprif.go

@@ -0,0 +1,231 @@
+// Mgmt
+// Copyright (C) James Shubin and the project contributors
+// Written by James Shubin <james@shubin.ca> and the project contributors
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+//
+// Additional permission under GNU GPL version 3 section 7
+//
+// If you modify this program, or any covered work, by linking or combining it
+// with embedded mcl code and modules (and that the embedded mcl code and
+// modules which link with this program, contain a copy of their source code in
+// the authoritative form) containing parts covered by the terms of any other
+// license, the licensors of this program grant you additional permission to
+// convey the resulting work. Furthermore, the licensors of this program grant
+// the original author, James Shubin, additional permission to update this
+// additional permission if he deems it necessary to achieve the goals of this
+// additional permission.
+
+package structs
+
+import (
+	"context"
+	"fmt"
+	"sync"
+
+	"github.com/purpleidea/mgmt/lang/interfaces"
+	"github.com/purpleidea/mgmt/lang/types"
+	"github.com/purpleidea/mgmt/pgraph"
+	"github.com/purpleidea/mgmt/util/errwrap"
+)
+
+const (
+	// ExprIfFuncName is the unique name identifier for this function.
+	ExprIfFuncName = "exprif"
+
+	// ExprIfFuncArgNameCondition is the name for the edge which connects
+	// the input condition to ExprIfFunc.
+	ExprIfFuncArgNameCondition = "condition"
+)
+
+// ExprIfFunc is a function that passes through the value of the correct branch
+// based on the conditional value it gets.
+type ExprIfFunc struct {
+	interfaces.Textarea
+
+	Type *types.Type // this is the type of the if expression output we hold
+
+	EdgeName string // name of the edge used
+
+	ThenGraph *pgraph.Graph
+	ElseGraph *pgraph.Graph
+
+	ThenFunc interfaces.Func
+	ElseFunc interfaces.Func
+
+	OutputVertex interfaces.Func
+
+	init *interfaces.Init
+	last *bool // last value received to use for diff
+}
+
+// String returns a simple name for this function. This is needed so this struct
+// can satisfy the pgraph.Vertex interface.
+func (obj *ExprIfFunc) String() string {
+	return ExprIfFuncName
+}
+
+// Validate tells us if the input struct takes a valid form.
+func (obj *ExprIfFunc) Validate() error {
+	if obj.Type == nil {
+		return fmt.Errorf("must specify a type")
+	}
+
+	if obj.EdgeName == "" {
+		return fmt.Errorf("must specify an edge name")
+	}
+
+	if obj.ThenFunc == nil {
+		return fmt.Errorf("must specify a then func")
+	}
+	if obj.ElseFunc == nil {
+		return fmt.Errorf("must specify an else func")
+	}
+
+	t1 := obj.ThenFunc.Info().Sig.Out
+	t2 := obj.ElseFunc.Info().Sig.Out
+	if err := t1.Cmp(t2); err != nil {
+		return errwrap.Wrapf(err, "types of exprif branches must match")
+	}
+
+	if obj.OutputVertex == nil {
+		return fmt.Errorf("the output vertex is missing")
+	}
+
+	return nil
+}
+
+// Info returns some static info about itself.
+func (obj *ExprIfFunc) Info() *interfaces.Info {
+	var typ *types.Type
+	if obj.Type != nil { // don't panic if called speculatively
+		typ = &types.Type{
+			Kind: types.KindFunc, // function type
+			Map: map[string]*types.Type{
+				ExprIfFuncArgNameCondition: types.TypeBool, // conditional must be a boolean
+			},
+			Ord: []string{ExprIfFuncArgNameCondition}, // conditional
+			Out: obj.Type,                             // result type must match
+		}
+	}
+
+	return &interfaces.Info{
+		Pure: true,
+		Memo: false, // TODO: ???
+		Sig:  typ,
+		Err:  obj.Validate(),
+	}
+}
+
+// Init runs some startup code for this if expression function.
+func (obj *ExprIfFunc) Init(init *interfaces.Init) error {
+	obj.init = init
+	return nil
+}
+
+// Stream takes an input struct in the format as described in the Func and Graph
+// methods of the Expr, and returns the actual expected value as a stream based
+// on the changing inputs to that value.
+func (obj *ExprIfFunc) Stream(ctx context.Context) error {
+	// XXX: is there a sync.Once sort of solution that would be more elegant here?
+	mutex := &sync.Mutex{}
+	done := false
+	send := func(ctx context.Context, b bool) error {
+		mutex.Lock()
+		defer mutex.Unlock()
+		if done {
+			return nil
+		}
+		done = true
+		defer close(obj.init.Output) // the sender closes
+
+		if !b {
+			return nil
+		}
+
+		// send dummy value to the output
+		select {
+		case obj.init.Output <- types.NewFloat(): // XXX: dummy value
+		case <-ctx.Done():
+			return ctx.Err()
+		}
+
+		return nil
+	}
+	defer send(ctx, false) // just close
+
+	defer func() {
+		obj.init.Txn.Reverse()
+	}()
+
+	for {
+		select {
+		case input, ok := <-obj.init.Input:
+			if !ok {
+				obj.init.Input = nil // block looping back here
+				if !done {
+					return fmt.Errorf("input closed without ever sending anything")
+				}
+				return nil
+			}
+
+			value, exists := input.Struct()[obj.EdgeName]
+			if !exists {
+				return fmt.Errorf("programming error, can't find edge")
+			}
+
+			b := value.Bool()
+
+			if obj.last != nil && *obj.last == b {
+				continue // result didn't change
+			}
+			obj.last = &b // store new result
+
+			if err := obj.replaceSubGraph(b); err != nil {
+				return errwrap.Wrapf(err, "could not replace subgraph")
+			}
+
+			send(ctx, true) // send dummy and then close
+
+			continue
+
+		case <-ctx.Done():
+			return nil
+		}
+	}
+}
+
+func (obj *ExprIfFunc) replaceSubGraph(b bool) error {
+	// delete the old subgraph
+	if err := obj.init.Txn.Reverse(); err != nil {
+		return errwrap.Wrapf(err, "could not Reverse")
+	}
+
+	var f interfaces.Func
+	if b {
+		obj.init.Txn.AddGraph(obj.ThenGraph)
+		f = obj.ThenFunc
+	} else {
+		obj.init.Txn.AddGraph(obj.ElseGraph)
+		f = obj.ElseFunc
+	}
+
+	// create the new subgraph
+	edgeName := OutputFuncArgName
+	edge := &interfaces.FuncEdge{Args: []string{edgeName}}
+	obj.init.Txn.AddVertex(f)
+	obj.init.Txn.AddEdge(f, obj.OutputVertex, edge)
+
+	return obj.init.Txn.Commit()
+}

lang/funcs/structs/if.go

@@ -1,167 +0,0 @@
-// Mgmt
-// Copyright (C) James Shubin and the project contributors
-// Written by James Shubin <james@shubin.ca> and the project contributors
-//
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// This program is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with this program.  If not, see <https://www.gnu.org/licenses/>.
-//
-// Additional permission under GNU GPL version 3 section 7
-//
-// If you modify this program, or any covered work, by linking or combining it
-// with embedded mcl code and modules (and that the embedded mcl code and
-// modules which link with this program, contain a copy of their source code in
-// the authoritative form) containing parts covered by the terms of any other
-// license, the licensors of this program grant you additional permission to
-// convey the resulting work. Furthermore, the licensors of this program grant
-// the original author, James Shubin, additional permission to update this
-// additional permission if he deems it necessary to achieve the goals of this
-// additional permission.
-
-package structs
-
-import (
-	"context"
-	"fmt"
-
-	"github.com/purpleidea/mgmt/lang/interfaces"
-	"github.com/purpleidea/mgmt/lang/types"
-)
-
-const (
-	// IfFuncName is the unique name identifier for this function.
-	IfFuncName = "if"
-)
-
-// IfFunc is a function that passes through the value of the correct branch
-// based on the conditional value it gets.
-type IfFunc struct {
-	interfaces.Textarea
-
-	Type *types.Type // this is the type of the if expression output we hold
-
-	init   *interfaces.Init
-	last   types.Value // last value received to use for diff
-	result types.Value // last calculated output
-}
-
-// String returns a simple name for this function. This is needed so this struct
-// can satisfy the pgraph.Vertex interface.
-func (obj *IfFunc) String() string {
-	return IfFuncName
-}
-
-// Validate tells us if the input struct takes a valid form.
-func (obj *IfFunc) Validate() error {
-	if obj.Type == nil {
-		return fmt.Errorf("must specify a type")
-	}
-	return nil
-}
-
-// Info returns some static info about itself.
-func (obj *IfFunc) Info() *interfaces.Info {
-	var typ *types.Type
-	if obj.Type != nil { // don't panic if called speculatively
-		typ = &types.Type{
-			Kind: types.KindFunc, // function type
-			Map: map[string]*types.Type{
-				"c": types.TypeBool, // conditional must be a boolean
-				"a": obj.Type,       // true branch must be this type
-				"b": obj.Type,       // false branch must be this type too
-			},
-			Ord: []string{"c", "a", "b"}, // conditional, and two branches
-			Out: obj.Type,                // result type must match
-		}
-	}
-
-	return &interfaces.Info{
-		Pure: true,
-		Memo: false, // TODO: ???
-		Sig:  typ,
-		Err:  obj.Validate(),
-	}
-}
-
-// Init runs some startup code for this if expression function.
-func (obj *IfFunc) Init(init *interfaces.Init) error {
-	obj.init = init
-	return nil
-}
-
-// Stream takes an input struct in the format as described in the Func and Graph
-// methods of the Expr, and returns the actual expected value as a stream based
-// on the changing inputs to that value.
-func (obj *IfFunc) Stream(ctx context.Context) error {
-	defer close(obj.init.Output) // the sender closes
-	for {
-		select {
-		case input, ok := <-obj.init.Input:
-			if !ok {
-				return nil // can't output any more
-			}
-			//if err := input.Type().Cmp(obj.Info().Sig.Input); err != nil {
-			//	return errwrap.Wrapf(err, "wrong function input")
-			//}
-			if obj.last != nil && input.Cmp(obj.last) == nil {
-				continue // value didn't change, skip it
-			}
-			obj.last = input // store for next
-
-			args, err := interfaces.StructToCallableArgs(input) // []types.Value, error)
-			if err != nil {
-				return err
-			}
-
-			result, err := obj.Call(ctx, args) // get the value...
-			if err != nil {
-				return err
-			}
-
-			// skip sending an update...
-			if obj.result != nil && result.Cmp(obj.result) == nil {
-				continue // result didn't change
-			}
-			obj.result = result // store new result
-
-		case <-ctx.Done():
-			return nil
-		}
-
-		select {
-		case obj.init.Output <- obj.result: // send
-			// pass
-		case <-ctx.Done():
-			return nil
-		}
-	}
-}
-
-// Call this function with the input args and return the value if it is possible
-// to do so at this time.
-// XXX: Is it correct to implement this here for this particular function?
-func (obj *IfFunc) Call(ctx context.Context, args []types.Value) (types.Value, error) {
-	if obj.Info() == nil {
-		return nil, fmt.Errorf("info is empty")
-	}
-	if obj.Info().Sig == nil {
-		return nil, fmt.Errorf("sig is empty")
-	}
-	if i, j := len(args), len(obj.Info().Sig.Ord); i != j {
-		return nil, fmt.Errorf("arg length doesn't match, got %d, exp: %d", i, j)
-	}
-
-	if c := args[0].Bool(); c {
-		return args[1], nil // true branch
-	}
-	return args[2], nil
-}

lang/interpret_test/TestAstFunc1/changing-func.txtar

@@ -21,25 +21,21 @@ $out2 = $fn2()
 test "${out1}" {}
 test "${out2}" {}
 -- OUTPUT --
-Edge: FuncValue -> if # a
-Edge: FuncValue -> if # a
-Edge: FuncValue -> if # b
-Edge: FuncValue -> if # b
 Edge: call -> callSubgraphOutput # dummy
 Edge: call -> callSubgraphOutput # dummy
-Edge: const: bool(false) -> if # c
-Edge: const: bool(true) -> if # c
-Edge: if -> call # fn
-Edge: if -> call # fn
-Vertex: FuncValue
-Vertex: FuncValue
-Vertex: FuncValue
-Vertex: FuncValue
+Edge: const: bool(false) -> exprif # condition
+Edge: const: bool(true) -> exprif # condition
+Edge: exprIfSubgraphOutput -> call # fn
+Edge: exprIfSubgraphOutput -> call # fn
+Edge: exprif -> exprIfSubgraphOutput # dummy
+Edge: exprif -> exprIfSubgraphOutput # dummy
 Vertex: call
 Vertex: call
 Vertex: callSubgraphOutput
 Vertex: callSubgraphOutput
 Vertex: const: bool(false)
 Vertex: const: bool(true)
-Vertex: if
-Vertex: if
+Vertex: exprIfSubgraphOutput
+Vertex: exprIfSubgraphOutput
+Vertex: exprif
+Vertex: exprif

lang/interpret_test/TestAstFunc1/importscope0.txtar

@@ -16,11 +16,9 @@ class xclass {
 	}
 }
 -- OUTPUT --
-Edge: const: str("bbb") -> if # a
-Edge: const: str("ccc") -> if # b
-Edge: os.is_family_debian -> if # c
-Vertex: const: str("bbb")
-Vertex: const: str("ccc")
+Edge: exprif -> exprIfSubgraphOutput # dummy
+Edge: os.is_family_debian -> exprif # condition
 Vertex: const: str("hello")
-Vertex: if
+Vertex: exprIfSubgraphOutput
+Vertex: exprif
 Vertex: os.is_family_debian

lang/interpret_test/TestAstFunc1/importscope2.txtar

@@ -15,11 +15,9 @@ class xclass {
 	}
 }
 -- OUTPUT --
-Edge: const: str("bbb") -> if # a
-Edge: const: str("ccc") -> if # b
-Edge: os.is_family_debian -> if # c
-Vertex: const: str("bbb")
-Vertex: const: str("ccc")
+Edge: exprif -> exprIfSubgraphOutput # dummy
+Edge: os.is_family_debian -> exprif # condition
 Vertex: const: str("hello")
-Vertex: if
+Vertex: exprIfSubgraphOutput
+Vertex: exprif
 Vertex: os.is_family_debian

lang/interpret_test/TestAstFunc1/shape8.txtar

@@ -18,17 +18,14 @@ $lambda = if $gt(10, 0) { # must be a const, otherwise this is a dynamic graph
 
 test [$lambda("hello"),] {}
 -- OUTPUT --
-Edge: FuncValue -> if # a
-Edge: FuncValue -> if # b
-Edge: _operator -> if # c
+Edge: _operator -> exprif # condition
 Edge: call -> callSubgraphOutput # dummy
 Edge: callSubgraphOutput -> composite: []str # 0
 Edge: const: int(0) -> _operator # b
 Edge: const: int(10) -> _operator # a
 Edge: const: str(">") -> _operator # op
-Edge: if -> call # fn
-Vertex: FuncValue
-Vertex: FuncValue
+Edge: exprIfSubgraphOutput -> call # fn
+Edge: exprif -> exprIfSubgraphOutput # dummy
 Vertex: _operator
 Vertex: call
 Vertex: callSubgraphOutput
@@ -37,4 +34,5 @@ Vertex: const: int(0)
 Vertex: const: int(10)
 Vertex: const: str(">")
 Vertex: const: str("hello")
-Vertex: if
+Vertex: exprIfSubgraphOutput
+Vertex: exprif