Skip to content

Commit

Permalink
Remove redundant fields from CallNode and BinaryNode
Browse files Browse the repository at this point in the history
  • Loading branch information
antonmedv committed Dec 29, 2023
1 parent 95ec882 commit f3a815c
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 184 deletions.
19 changes: 7 additions & 12 deletions ast/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ast

import (
"reflect"
"regexp"

"github.com/expr-lang/expr/file"
)
Expand Down Expand Up @@ -104,10 +103,9 @@ type UnaryNode struct {
// BinaryNode represents a binary operator.
type BinaryNode struct {
base
Operator string // Operator of the binary operator. Like "+" in "foo + bar" or "matches" in "foo matches bar".
Left Node // Left node of the binary operator.
Right Node // Right node of the binary operator.
Regexp *regexp.Regexp // Internal. Regexp of the "matches" operator. Like "f.+".
Operator string // Operator of the binary operator. Like "+" in "foo + bar" or "matches" in "foo matches bar".
Left Node // Left node of the binary operator.
Right Node // Right node of the binary operator.
}

// ChainNode represents an optional chaining group.
Expand Down Expand Up @@ -151,20 +149,17 @@ type SliceNode struct {
// CallNode represents a function or a method call.
type CallNode struct {
base
Callee Node // Node of the call. Like "foo" in "foo()".
Arguments []Node // Arguments of the call.
Typed int // Internal. Used to indicate compiler what type is one of vm.FuncTypes.
Fast bool // Internal. Used to indicate compiler what this call is a fast call.
Func *Function // Internal. Used to pass function information from type checker to compiler.
Callee Node // Node of the call. Like "foo" in "foo()".
Arguments []Node // Arguments of the call.
}

// BuiltinNode represents a builtin function call.
type BuiltinNode struct {
base
Name string // Name of the builtin function. Like "len" in "len(foo)".
Arguments []Node // Arguments of the builtin function.
Throws bool // Internal. If true then accessing a field or array index can throw an error. Used by optimizer.
Map Node // Internal. Used by optimizer to fold filter() and map() builtins.
Throws bool // If true then accessing a field or array index can throw an error. Used by optimizer.
Map Node // Used by optimizer to fold filter() and map() builtins.
}

// ClosureNode represents a predicate.
Expand Down
74 changes: 8 additions & 66 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/expr-lang/expr/conf"
"github.com/expr-lang/expr/file"
"github.com/expr-lang/expr/parser"
"github.com/expr-lang/expr/vm"
)

func Check(tree *parser.Tree, config *conf.Config) (t reflect.Type, err error) {
Expand Down Expand Up @@ -374,11 +373,10 @@ func (v *checker) BinaryNode(node *ast.BinaryNode) (reflect.Type, info) {

case "matches":
if s, ok := node.Right.(*ast.StringNode); ok {
r, err := regexp.Compile(s.Value)
_, err := regexp.Compile(s.Value)
if err != nil {
return v.error(node, err.Error())
}
node.Regexp = r
}
if isString(l) && isString(r) {
return boolType, info{}
Expand Down Expand Up @@ -549,7 +547,6 @@ func (v *checker) functionReturnType(node *ast.CallNode) (reflect.Type, info) {
fn, fnInfo := v.visit(node.Callee)

if fnInfo.fn != nil {
node.Func = fnInfo.fn
return v.checkFunction(fnInfo.fn, node, node.Arguments)
}

Expand All @@ -571,33 +568,13 @@ func (v *checker) functionReturnType(node *ast.CallNode) (reflect.Type, info) {
case reflect.Interface:
return anyType, info{}
case reflect.Func:
inputParamsCount := 1 // for functions
if fnInfo.method {
inputParamsCount = 2 // for methods
}
// TODO: Deprecate OpCallFast and move fn(...any) any to TypedFunc list.
// To do this we need add support for variadic arguments in OpCallTyped.
if !isAny(fn) &&
fn.IsVariadic() &&
fn.NumIn() == inputParamsCount &&
fn.NumOut() == 1 &&
fn.Out(0).Kind() == reflect.Interface {
rest := fn.In(fn.NumIn() - 1) // function has only one param for functions and two for methods
if kind(rest) == reflect.Slice && rest.Elem().Kind() == reflect.Interface {
node.Fast = true
}
}

outType, err := v.checkArguments(fnName, fn, fnInfo.method, node.Arguments, node)
if err != nil {
if v.err == nil {
v.err = err
}
return anyType, info{}
}

v.findTypedFunc(node, fn, fnInfo.method)

return outType, info{}
}
return v.error(node, "%v is not callable", fn)
Expand Down Expand Up @@ -883,7 +860,13 @@ func (v *checker) checkFunction(f *ast.Function, node ast.Node, arguments []ast.
return v.error(node, "no matching overload for %v", f.Name)
}

func (v *checker) checkArguments(name string, fn reflect.Type, method bool, arguments []ast.Node, node ast.Node) (reflect.Type, *file.Error) {
func (v *checker) checkArguments(
name string,
fn reflect.Type,
method bool,
arguments []ast.Node,
node ast.Node,
) (reflect.Type, *file.Error) {
if isAny(fn) {
return anyType, nil
}
Expand Down Expand Up @@ -1122,44 +1105,3 @@ func (v *checker) PairNode(node *ast.PairNode) (reflect.Type, info) {
v.visit(node.Value)
return nilType, info{}
}

func (v *checker) findTypedFunc(node *ast.CallNode, fn reflect.Type, method bool) {
// OnCallTyped doesn't work for functions with variadic arguments,
// and doesn't work named function, like `type MyFunc func() int`.
// In PkgPath() is an empty string, it's unnamed function.
if !fn.IsVariadic() && fn.PkgPath() == "" {
fnNumIn := fn.NumIn()
fnInOffset := 0
if method {
fnNumIn--
fnInOffset = 1
}
funcTypes:
for i := range vm.FuncTypes {
if i == 0 {
continue
}
typed := reflect.ValueOf(vm.FuncTypes[i]).Elem().Type()
if typed.Kind() != reflect.Func {
continue
}
if typed.NumOut() != fn.NumOut() {
continue
}
for j := 0; j < typed.NumOut(); j++ {
if typed.Out(j) != fn.Out(j) {
continue funcTypes
}
}
if typed.NumIn() != fnNumIn {
continue
}
for j := 0; j < typed.NumIn(); j++ {
if typed.In(j) != fn.In(j+fnInOffset) {
continue funcTypes
}
}
node.Typed = i
}
}
}
54 changes: 7 additions & 47 deletions checker/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,11 @@ unknown pointer #unknown (1:11)
cannot use int as type string in array (1:4)
| 42 in ["a", "b", "c"]
| ...^
"foo" matches "[+"
error parsing regexp: missing closing ]: ` + "`[+`" + ` (1:7)
| "foo" matches "[+"
| ......^
`

func TestCheck_error(t *testing.T) {
Expand Down Expand Up @@ -777,49 +782,6 @@ func TestCheck_TypeWeights(t *testing.T) {
}
}

func TestCheck_CallFastTyped(t *testing.T) {
env := map[string]any{
"fn": func([]any, string) string {
return "foo"
},
}

tree, err := parser.Parse("fn([1, 2], 'bar')")
require.NoError(t, err)

_, err = checker.Check(tree, conf.New(env))
require.NoError(t, err)

require.False(t, tree.Node.(*ast.CallNode).Fast)
require.Equal(t, 22, tree.Node.(*ast.CallNode).Typed)
}

func TestCheck_CallFastTyped_Method(t *testing.T) {
env := mock.Env{}

tree, err := parser.Parse("FuncTyped('bar')")
require.NoError(t, err)

_, err = checker.Check(tree, conf.New(env))
require.NoError(t, err)

require.False(t, tree.Node.(*ast.CallNode).Fast)
require.Equal(t, 42, tree.Node.(*ast.CallNode).Typed)
}

func TestCheck_CallTyped_excludes_named_functions(t *testing.T) {
env := mock.Env{}

tree, err := parser.Parse("FuncNamed('bar')")
require.NoError(t, err)

_, err = checker.Check(tree, conf.New(env))
require.NoError(t, err)

require.False(t, tree.Node.(*ast.CallNode).Fast)
require.Equal(t, 0, tree.Node.(*ast.CallNode).Typed)
}

func TestCheck_works_with_nil_types(t *testing.T) {
env := map[string]any{
"null": nil,
Expand Down Expand Up @@ -908,8 +870,7 @@ func TestCheck_Function_types_are_checked(t *testing.T) {

_, err = checker.Check(tree, config)
require.NoError(t, err)
require.NotNil(t, tree.Node.(*ast.CallNode).Func)
require.Equal(t, "add", tree.Node.(*ast.CallNode).Func.Name)
require.Equal(t, reflect.Int, tree.Node.Type().Kind())
})
}

Expand Down Expand Up @@ -943,8 +904,7 @@ func TestCheck_Function_without_types(t *testing.T) {

_, err = checker.Check(tree, config)
require.NoError(t, err)
require.NotNil(t, tree.Node.(*ast.CallNode).Func)
require.Equal(t, "add", tree.Node.(*ast.CallNode).Func.Name)
require.Equal(t, reflect.Interface, tree.Node.Type().Kind())
}

func TestCheck_dont_panic_on_nil_arguments_for_builtins(t *testing.T) {
Expand Down
78 changes: 78 additions & 0 deletions checker/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/expr-lang/expr/ast"
"github.com/expr-lang/expr/conf"
"github.com/expr-lang/expr/vm"
)

func FieldIndex(types conf.TypesTable, node ast.Node) (bool, []int, string) {
Expand Down Expand Up @@ -48,3 +49,80 @@ func MethodIndex(types conf.TypesTable, node ast.Node) (bool, int, string) {
}
return false, 0, ""
}

func TypedFuncIndex(fn reflect.Type, method bool) (int, bool) {
if fn == nil {
return 0, false
}
if fn.Kind() != reflect.Func {
return 0, false
}
// OnCallTyped doesn't work for functions with variadic arguments.
if fn.IsVariadic() {
return 0, false
}
// OnCallTyped doesn't work named function, like `type MyFunc func() int`.
if fn.PkgPath() != "" { // If PkgPath() is not empty, it means that function is named.
return 0, false
}

fnNumIn := fn.NumIn()
fnInOffset := 0
if method {
fnNumIn--
fnInOffset = 1
}

funcTypes:
for i := range vm.FuncTypes {
if i == 0 {
continue
}
typed := reflect.ValueOf(vm.FuncTypes[i]).Elem().Type()
if typed.Kind() != reflect.Func {
continue
}
if typed.NumOut() != fn.NumOut() {
continue
}
for j := 0; j < typed.NumOut(); j++ {
if typed.Out(j) != fn.Out(j) {
continue funcTypes
}
}
if typed.NumIn() != fnNumIn {
continue
}
for j := 0; j < typed.NumIn(); j++ {
if typed.In(j) != fn.In(j+fnInOffset) {
continue funcTypes
}
}
return i, true
}
return 0, false
}

func IsFastFunc(fn reflect.Type, method bool) bool {
if fn == nil {
return false
}
if fn.Kind() != reflect.Func {
return false
}
numIn := 1
if method {
numIn = 2
}
if !isAny(fn) &&
fn.IsVariadic() &&
fn.NumIn() == numIn &&
fn.NumOut() == 1 &&
fn.Out(0).Kind() == reflect.Interface {
rest := fn.In(fn.NumIn() - 1) // function has only one param for functions and two for methods
if kind(rest) == reflect.Slice && rest.Elem().Kind() == reflect.Interface {
return true
}
}
return false
}
27 changes: 27 additions & 0 deletions checker/info_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package checker_test

import (
"reflect"
"testing"

"github.com/stretchr/testify/require"

"github.com/expr-lang/expr/checker"
"github.com/expr-lang/expr/test/mock"
)

func TestTypedFuncIndex(t *testing.T) {
fn := func([]any, string) string {
return "foo"
}
index, ok := checker.TypedFuncIndex(reflect.TypeOf(fn), false)
require.True(t, ok)
require.Equal(t, 22, index)
}

func TestTypedFuncIndex_excludes_named_functions(t *testing.T) {
var fn mock.MyFunc

_, ok := checker.TypedFuncIndex(reflect.TypeOf(fn), false)
require.False(t, ok)
}
Loading

0 comments on commit f3a815c

Please sign in to comment.