Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Struct fields implementing starlark.Value are not passed through verbatim as starlark.Value #12

Open
jazzy-crane opened this issue Jan 7, 2019 · 1 comment

Comments

@jazzy-crane
Copy link

jazzy-crane commented Jan 7, 2019

I'm not sure how best to explain this, but I'll give it a go. Please ping back if you need more details.

I'm looking at using starlight against some Go structs generated from proto definitions by protoc.

As an example:

enum BarEnum {
    BAR_UNKNOWN = 0
    BAR_ONE = 1
    BAR_TWO = 2
}

message Foo {
    string Msg = 1
    BarEnum Bar = 2;
}

I would then pass in the values of BarEnum in my

testFoo := Foo { Msg: "Hello World", Bar: BarEnum_BAR_ONE }
var env map[string]interface{}
env["testFoo"] = testFoo
env["BAR_UNKNOWN"] = BarEnum_BAR_UNKNOWN
env["BAR_ONE"] = BarEnum_BAR_ONE
env["BAR_TWO"] = BarEnum_BAR_TWO

starScipt = `
func(e):
    return e.Bar == BAR_ONE
return func(testFoo)
`
starlight.Eval([]byte(starScript), env, nil)
...
get result
...

Unfortunately the function is returning false. Digging deeper it seems that what protoc generates for BarEnum looks something like this:

type BarEnum int32

const (
	BarEnum_BAR_UNKNOWN BarEnum = 0
	BarEnum_BAR_ONE BarEnum = 1
	BarEnum_BAR_TWO BarEnum = 2
)

var BarEnum_name = map[int32]string{
	0: "BAR_UNKNOWN",
	1: "BAR_ONE",
	2: "BAR_TWO",
}

var BarEnum_value = map[string]int32{
	"BAR_UNKNOWN":  0,
	"BAR_ONE": 1,
	"BAR_TWO": 2,
}

func (x BarEnum) String() string {
	return proto.EnumName(BarEnum_name, int32(x))
}

func (BarEnum) EnumDescriptor() ([]byte, []int) {
	return fileDescriptor_14343df069b9efbf, []int{0}
}

while the maps are useful to iterate to export the enum values, the fact that BarEnum is a basic type with functions on it causes it to be exported as a GoInterface, and so not comparable as the base type.

This is fine. I noticed that if the type implements starlark.Value it can be passed through verbatim. I did this (adding Type, Freeze, Truth, Hash and CompareSameType implementations to BarEnum) and found that the enum values I was exporting were now comparable, but they still didn't compare properly with the Bar field of the Foo struct.

Digging a little deeper, it seems for member fields of structs you call toValue() for the field. This skips the 'does it implement starlark.Value, then pass it through verbatim' check. My case seems to work if I change the line:

        if field.Kind() != reflect.Invalid {
-               return toValue(field)
+               return ToValue(field.Interface())
        }
        return nil, nil

Is this fine to do or does it break other cases?

@jazzy-crane jazzy-crane changed the title Nested values implementing starlark.Value Struct fields implementing starlark.Value are not passed through verbatime as starlark.Value Jan 7, 2019
@jazzy-crane jazzy-crane changed the title Struct fields implementing starlark.Value are not passed through verbatime as starlark.Value Struct fields implementing starlark.Value are not passed through verbatim as starlark.Value Jan 7, 2019
@jazzy-crane
Copy link
Author

This also happens if you convert.ToValue a map[string]starlark.Value - the starlark.Value values get wrapped in a starlight interface. Same issue with toValue and ToValue

chasehensel added a commit to chasehensel/starlight that referenced this issue Jul 6, 2020
RPC, Updates, Starlark improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant