Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

1.15.4 breaks function patcher, with error: expected float64, but got interface {} #473

Answered by antonmedv
tw1nk asked this question in Q&A
Discussion options

In our codebase we have a Patcher that let's us not specify the context as a function value if the function requires a context.

However when we compile scripts with the 1.15.4 version we get errors similar to: expected float64, but got interface {}

Embedded test:


import (
	"context"
	"fmt"
	"reflect"
	"strconv"
	"testing"
	"github.com/antonmedv/expr"
	"github.com/antonmedv/expr/ast"
)
type TestStruct struct {
	Data string
}
func (ts *TestStruct) GetFloat() (float64, error) {
	return strconv.ParseFloat(ts.Data, 64)
}
func testMeFunc(arguments ...any) (any, error) {
	if len(arguments) != 2 {
		return nil, fmt.Errorf("expected 2 argument. got: %d", len(arguments))
	}
	ctx, ok := arguments[0].(context.Context)
	if !ok {
		return nil, fmt.Errorf("exprGetCurveTable() first argument should be context")
	}
	// imagine we need something from the context.
	ctxValue := ctx.Value("test").(string)
	if ctxValue != "value" {
		return nil, fmt.Errorf("invalid context value")
	}
	data, ok := arguments[1].(string)
	if !ok {
		return nil, fmt.Errorf("expected argument 2 to be a string. was: %T", arguments[0])
	}
	return &TestStruct{
		Data: data,
	}, nil
}
type EvaluationData struct {
	Ctx context.Context `expr:"ctx"`
}
type ContextPatcher struct{}
func (p *ContextPatcher) Visit(node *ast.Node) {
	contextType := reflect.TypeOf((*context.Context)(nil)).Elem()
	//nolint:nestif
	if callNode, ok := (*node).(*ast.CallNode); ok {
		// patch context calls so we don't have to pass the context in the code
		if callNode.Func == nil {
			return
		}
		funcType := callNode.Func.Types[0]
		if funcType.NumIn() > 0 {
			firstIn := funcType.In(0)
			if firstIn.Implements(contextType) {
				// prepend patch context calls so we don't need to specify the context in the function invocation
				args := append([]ast.Node{
					&ast.IdentifierNode{
						Value: "ctx",
					},
				}, callNode.Arguments...)
				newNode := &ast.CallNode{
					Callee: callNode.Callee,
					Arguments: args,
					Typed: callNode.Typed,
					Fast: callNode.Fast,
					Func: callNode.Func,
				}
				ast.Patch(node, newNode)
			}
		}
	}
}
func TestFunctionCallReturnsFloat(t *testing.T) {
	opts := []expr.Option{
		expr.Function("TestMe",
			testMeFunc,
			new(func(context.Context, string) *TestStruct),
		),
		expr.Env(&EvaluationData{}),
		expr.AsFloat64(),
		expr.Patch(&ContextPatcher{}),
	}
	program, err := expr.Compile(`TestMe("1.33").GetFloat()`, opts...)
	if err != nil {
		t.Fatalf("failed to compile script. %v", err)
	}
	ctx := context.WithValue(context.Background(), "test", "value")
	evalData := &EvaluationData{
		Ctx: ctx,
	}
	result, err := expr.Run(program, evalData)
	if err != nil {
		t.Fatalf("failed to run program. %v", err)
	}
	value, ok := result.(float64)
	if !ok {
		t.Errorf("expected result to be a float. was: %T", result)
	}
	if value != 1.33 {
		t.Errorf("expected result to be 1.33, was: %v", value)
	}
}
You must be logged in to vote

expected float64, but got interface {}

First about this issue. Before 1.15.4, AsFloat64() would accept float64 || any. Now it is more explicit, and accepts only float64.

You need to add AsAny():

	opts := []expr.Option{
		expr.AsFloat64(),
+		expr.AsAny(),
	}

More info in #347.

We have overriden the binary operators (&&, ||) in our code. For simple expression (Func1() || Func2()) it is working as expected, but when we introduce a complex(not so) expression (Func1() || Func2() || Func3()), it is erroring out.

Without code, it is unclear. I maybe some bug in your Patcher implementation. Tr to dump ast after patching.

Replies: 4 comments 11 replies

Comment options

+1 We have overriden the binary operators (&&, ||) in our code. For simple expression (Func1() || Func2()) it is working as expected, but when we introduce a complex(not so) expression (Func1() || Func2() || Func3()), it is erroring out.

interface conversion: interface {} is CustomEntity, not bool (1:180)

You must be logged in to vote
1 reply
Comment options

I've fixed the error. Now original code also works. See #548

Comment options

expected float64, but got interface {}

First about this issue. Before 1.15.4, AsFloat64() would accept float64 || any. Now it is more explicit, and accepts only float64.

You need to add AsAny():

	opts := []expr.Option{
		expr.AsFloat64(),
+		expr.AsAny(),
	}

More info in #347.

We have overriden the binary operators (&&, ||) in our code. For simple expression (Func1() || Func2()) it is working as expected, but when we introduce a complex(not so) expression (Func1() || Func2() || Func3()), it is erroring out.

Without code, it is unclear. I maybe some bug in your Patcher implementation. Tr to dump ast after patching.

You must be logged in to vote
4 replies
Comment options

Without code, it is unclear. I maybe some bug in your Patcher implementation. Tr to dump ast after patching.

This is a sample version of the code that we use

package main
import (
	"fmt"
	"github.com/antonmedv/expr"
	"github.com/antonmedv/expr/ast"
)
type RuleEvaluation struct {
	Field1 string
	Field2 int
}
type Env struct {
	Evaluator *Evaluator
}
func (Env) RuleEvaluationOr(a, b *RuleEvaluation) *RuleEvaluation {
	return a
}
func (Env) RuleEvaluationAnd(a, b *RuleEvaluation) *RuleEvaluation {
	return b
}
type Evaluator struct {
	Field1 string
	Field2 int
}
func (e Env) Function1(a int, b string) *RuleEvaluation {
	// read Evaluator struct values and compute response
	return &RuleEvaluation {
		Field1: e.Evaluator.Field1 + b,
		Field2: e.Evaluator.Field2 + a,
	}
}
func main() {
	evaluator := &Evaluator {
		Field1: "abc",
		Field2: 1,
	}
	env := Env {
		Evaluator: evaluator,
	}
	option := []expr.Option{
		expr.AsKind(reflect.TypeOf(&RuleEvaluation{}).Kind()),
		expr.Env(env),
		expr.Operator("||", "RuleEvaluationOr"),
		expr.Operator("&&", "RuleEvaluationAnd"),
	}
	// expression := "Function1(1, \"hello\") || Function1(2, \"hi\")" // works
	expression := "Function1(1, \"hello\") || Function1(2, \"hi\") || Function1(3, \"world\")" // fails
	program, err := expr.Compile(expression, option...)
	if err != nil {
		fmt.Println("complie error ", err.Error())
		return
	}
	expressionResult, err := expr.Run(program, env)
	if err != nil {
		fmt.Println("expr execution error ", err.Error())
		return
	}
	result := expressionResult.(*RuleEvaluation)
	fmt.Println(result)
}

I am getting the following error

expr execution error interface conversion: interface {} is main.RuleEvaluation, not bool (1:45)
 | Function1(1, "hello") || Function1(2, "hi") || Function1(3, "world")
 | ............................................^
Comment options

You need to add AsAny():

The script and code it calls in my example is explicitly returning a float, not an any, so I don't understand why we need to add an AsAny, as that would lessen the type safety?

Comment options

I think the problem is with the operator overloading.
I slightly modified the operator overloading example (https://expr.medv.io/docs/Operator-Overloading) to overload || operator, got same kind of error

package main
import (
	"fmt"
	"github.com/antonmedv/expr"
)
func main() {
	code := `Now() || Now() || CreatedAt`
	options := []expr.Option{
		expr.Env(Env{}),
		expr.Operator("||", "Sub"), // Replace `-` operator with function `Sub`.
	}
	program, err := expr.Compile(code, options...)
	if err != nil {
		panic(err)
	}
	env := Env{
		CreatedAt: 50,
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}
	fmt.Print(output)
}
type Env struct {
	CreatedAt int
}
func (Env) Now() int { return 10 }
func (Env) Sub(a, b int) int { return a - b }

response

panic: interface conversion: interface {} is int, not bool (1:16)
 | Now() || Now() || CreatedAt
 | ...............^
Comment options

I've fixed the error. Now original code also works. See #548

Answer selected by antonmedv
Comment options

After a long debugging session I figured out that the checker.Check modifies the ast state by setting the nodeType.

In my patcher I don't want my users to have to pass the context argument to the functions we call as that isn't ergonomic or necessary for the scripters to have to know if the function call requires an context argument or not.

From my example above our users would input: TestMe("1.33").GetFloat() but the actual script that the patcher generates will be TestMe(ctx, "1.33").GetFloat()

So when the checker runs at https://github.com/antonmedv/expr/blob/939aca18e66514ca0727c309f8df8c510a0e2fc1/expr.go#L182C1-L182C8
it will set the return value of the TestMe function to the invalid value interface{} instead of the correct value *scripting.TestStruct as the script is syntactically correct but fails to the checker before we run the patcher.

The change to checker.CallNode introduced in fcf2b55
in combination with
https://github.com/antonmedv/expr/blob/939aca18e66514ca0727c309f8df8c510a0e2fc1/checker/checker.go#L135

causes the issue I have.

Maybe what I need is to have another visitor that runs before performing the typecheck to modify the AST for my usecase?

You must be logged in to vote
4 replies
Comment options

Wow! This is a complex issue. I think you can try to set type from your patcher: node.SetType(). Let's add a test for this case in Expr.

Comment options

I got it to work correctly by explicitly call (*node).SetType(nil) for all nodes in my patcher.

it's a bit weird but at least it solves my issue

type ContextPatcher struct{}
func (p *ContextPatcher) Visit(node *ast.Node) {
	(*node).SetType(nil)
	contextType := reflect.TypeOf((*context.Context)(nil)).Elem()
	//nolint:nestif
	if callNode, ok := (*node).(*ast.CallNode); ok {
		// patch context calls so we don't have to pass the context in the code
		if callNode.Func == nil {
			return
		}
		funcType := callNode.Func.Types[0]
		if funcType.NumIn() > 0 {
			firstIn := funcType.In(0)
			if firstIn.Implements(contextType) {
				// prepend patch context calls so we don't need to specify the context in the function invocation
				args := append([]ast.Node{
					&ast.IdentifierNode{
						Value: "ctx",
					},
				}, callNode.Arguments...)
				newNode := &ast.CallNode{
					Callee: callNode.Callee,
					Arguments: args,
					Typed: callNode.Typed,
					Fast: callNode.Fast,
					Func: callNode.Func,
				}
				newNode.SetType(funcType.Out(0))
				ast.Patch(node, newNode)
			}
		}
	}
}
Comment options

Actually I think calling '(*node).SetType(nil)' might be required for everyone using a patcher. Including operator overloads.

Comment options

FYI. I've implemented my own version of context patcher and added a config option https://github.com/antonmedv/expr/blob/f92bb7949de064a05d5b3a61d1b07abef0c521bb/expr.go#L157-L161

func ExampleWithContext() {
	env := map[string]any{
		"fn": func(ctx context.Context, _, _ int) int {
			// An infinite loop that can be canceled by context.
			for {
				select {
				case <-ctx.Done():
					return 42
				}
			}
		},
		"ctx": context.TODO(), // Context should be passed as a variable.
	}
	program, err := expr.Compile(`fn(1, 2)`,
		expr.Env(env),
		expr.WithContext("ctx"), // Pass context variable name.
	)
	if err != nil {
		fmt.Printf("%v", err)
		return
	}
	// Cancel context after 100 milliseconds.
	ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100)
	defer cancel()
	// After program is compiled, context can be passed to Run.
	env["ctx"] = ctx
	// Run will return 42 after 100 milliseconds.
	output, err := expr.Run(program, env)
	if err != nil {
		fmt.Printf("%v", err)
		return
	}
	fmt.Printf("%v", output)
	// Output: 42
}
Comment options

Any tips for fixing this error

I slightly modified the operator overloading example (https://expr.medv.io/docs/Operator-Overloading) to overload || operator

Tried SetType using patcher still did not help

package main
import (
	"fmt"
	"github.com/antonmedv/expr"
)
func main() {
	code := `Now() || Now() || CreatedAt`
	options := []expr.Option{
		expr.Env(Env{}),
		expr.Operator("||", "Sub"), // Replace `-` operator with function `Sub`.
	}
	program, err := expr.Compile(code, options...)
	if err != nil {
		panic(err)
	}
	env := Env{
		CreatedAt: 50,
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}
	fmt.Print(output)
}
type Env struct {
	CreatedAt int
}
func (Env) Now() int { return 10 }
func (Env) Sub(a, b int) int { return a - b }

response

panic: interface conversion: interface {} is int, not bool (1:16)
 | Now() || Now() || CreatedAt
 | ...............^
You must be logged in to vote
2 replies
Comment options

@rajagopal-chinnaraj
I think you might need to patch the return value of Callee (with the SetType function).
I'm not sure how you would do that in the current API though, without going through a Patch function

Comment options

I fixed the error! Now original code also works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
Q&A
Labels
None yet
Converted from issue

This discussion was converted from issue #472 on November 21, 2023 18:05.

AltStyle によって変換されたページ (->オリジナル) /