Skip to content

Commit

Permalink
Allow passing nil to functions as nil interface{} (#87)
Browse files Browse the repository at this point in the history
* Update vm.go

golang gotcha: reflect calls with nil params fail without this trick

* Update vm.go

* Problem with passing "nil" when operators are overloaded with interface{} type parameters fixed

e.g.
a != nil
was not working when "!=" is overloaded with
func notEqual(a, b interface{}) interface{} {...}

* Compile fix

Forgot to close ")"

* Added test that illustrates what did not work when using overloaded operators and passing nil

* Updated test for overloaded eaqal with using nil

a == b replaced with a == nil to illustrated the problem when using nil directly in expression

* Fixed the problem for untyped nil

We use the trick only for untyped nil:
https://play.golang.org/p/MyAztoZaqhx

* Refactor call procedure

Co-authored-by: wintermute-cds <42935612+wintermute-cds@users.noreply.github.com>
  • Loading branch information
antonmedv and wintermute-cds committed Feb 20, 2020
1 parent daa3bfc commit ab801d4
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
4 changes: 4 additions & 0 deletions checker/checker.go
Expand Up @@ -409,6 +409,10 @@ func (v *visitor) checkFunc(fn reflect.Type, method bool, node ast.Node, name st
setTypeForIntegers(arg, t)
}

if t == nil {
continue
}

if !t.AssignableTo(in) {
panic(v.error(arg, "cannot use %v as argument (type %v) to call %v ", t, in, name))
}
Expand Down
21 changes: 20 additions & 1 deletion expr_test.go
Expand Up @@ -873,10 +873,29 @@ func TestExpr_map_default_values_compile_check(t *testing.T) {
}
}

func TestExpr_calls_with_nil(t *testing.T) {
env := map[string]interface{}{
"equals": func(a, b interface{}) interface{} {
return a == b
},
}

p, err := expr.Compile(
"a == nil && equals(b, nil)",
expr.Env(env),
expr.Operator("==", "equals"),
expr.AllowUndefinedVariables(),
)
require.NoError(t, err)

out, err := expr.Run(p, env)
require.NoError(t, err)
require.Equal(t, true, out)
}

//
// Mock types
//

type mockEnv struct {
Any interface{}
Int, One, Two, Three int
Expand Down
4 changes: 2 additions & 2 deletions internal/conf/operators_table.go
Expand Up @@ -16,8 +16,8 @@ func FindSuitableOperatorOverload(fns []string, types TypesTable, l, r reflect.T
firstArgType := fnType.Type.In(firstInIndex)
secondArgType := fnType.Type.In(firstInIndex + 1)

firstArgumentFit := l == firstArgType || (firstArgType.Kind() == reflect.Interface && l.Implements(firstArgType))
secondArgumentFit := r == secondArgType || (secondArgType.Kind() == reflect.Interface && r.Implements(secondArgType))
firstArgumentFit := l == firstArgType || (firstArgType.Kind() == reflect.Interface && (l == nil || l.Implements(firstArgType)))
secondArgumentFit := r == secondArgType || (secondArgType.Kind() == reflect.Interface && (r == nil || r.Implements(secondArgType)))
if firstArgumentFit && secondArgumentFit {
return fnType.Type.Out(0), fn, true
}
Expand Down
16 changes: 14 additions & 2 deletions vm/vm.go
Expand Up @@ -250,7 +250,13 @@ func (vm *VM) Run(program *Program, env interface{}) interface{} {
call := vm.constant().(Call)
in := make([]reflect.Value, call.Size)
for i := call.Size - 1; i >= 0; i-- {
in[i] = reflect.ValueOf(vm.pop())
param := vm.pop()
if param == nil {
// In case of nil interface{} (nil type) use this hack,
// otherwise reflect.Call will panic on zero value.
param = reflect.ValueOf(&in).Elem()
}
in[i] = reflect.ValueOf(param)
}
out := fetchFn(env, call.Name).Call(in)
vm.push(out[0].Interface())
Expand All @@ -268,7 +274,13 @@ func (vm *VM) Run(program *Program, env interface{}) interface{} {
call := vm.constants[vm.arg()].(Call)
in := make([]reflect.Value, call.Size)
for i := call.Size - 1; i >= 0; i-- {
in[i] = reflect.ValueOf(vm.pop())
param := vm.pop()
if param == nil {
// In case of nil interface{} (nil type) use this hack,
// otherwise reflect.Call will panic on zero value.
param = reflect.ValueOf(&in).Elem()
}
in[i] = reflect.ValueOf(param)
}
out := fetchFn(vm.pop(), call.Name).Call(in)
vm.push(out[0].Interface())
Expand Down

0 comments on commit ab801d4

Please sign in to comment.