-
Notifications
You must be signed in to change notification settings - Fork 192
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
Polymorphism support in generator #184
Comments
Yeah, I've run into this (or something like it) before, but I've never put together a solution. For what it's worth, |
That's the way I was doing it, I just have a few too many spots that need it now. At the moment I'm just hand-generating intermediate types that serialise the kind/data but it's pretty janky and it's not transparent at all, my app gets filled with junk where I have to traverse the intermediate type. I was thinking some support in the generator for a common pattern of packing the type and data as a 2-tuple transparently would be quite useful - this is a really common thing to come up in serialisation libraries and I haven't seen it handled anything close to well anywhere except gob, but you know... unless you like everything being four orders of magnitude slower than anything else gob is of course an obvious non-starter! It would also save me (and, especially, any developers who have to deal with this app after me) a bucketload of pain to have it properly supported! I'm happy to whip up a draft if you're up for exploring the idea further. I'm figuring if anything was even going to be accepted at all we'd have to kick it around a bit to get something acceptable, that's no problem at my end. |
Another approach would be to handle the polymorphism inside a preSave/postLoad hook; I added these a while back when I needed them. cf https://github.com/glycerine/greenpack/blob/master/msgp/read.go#L13 |
For what it's worth, this PR (after resolving minor conflicts) still holds up. The boilerplate code required can easily be auto-generated if two functions are provided, one to map struct to name and one to map name to struct. Those functions could also be generated with another tool that works at the package level, since it would have to scan the entire package to find all structs that implement some interface. package main
import (
"bytes"
"fmt"
"go/types"
"io/ioutil"
"strings"
"golang.org/x/tools/go/packages"
)
func getPackage(name string) *packages.Package {
cfg := &packages.Config{Mode: packages.NeedName | packages.NeedTypes | packages.NeedImports}
pkgs, err := packages.Load(cfg, name)
if err != nil {
panic(err)
}
return pkgs[0]
}
func main() {
modelsPkg := getPackage("hardcode or get from argv")
var structs []types.Object
interfaceToStruct := map[string][]string{}
interfaces := map[string]*types.Interface{}
// Find all structs defined
scope := modelsPkg.Types.Scope()
for _, name := range scope.Names() {
obj := scope.Lookup(name)
if _, ok := obj.(*types.TypeName); !ok {
// Not a named type
continue
}
_, isStruct := obj.Type().Underlying().(*types.Struct)
_, isInterface := obj.Type().Underlying().(*types.Interface)
if !isStruct && !isInterface {
// Not a struct or interface
continue
}
name := obj.Name()
if isInterface {
interfaces[name] = obj.Type().Underlying().(*types.Interface)
} else {
structs = append(structs, obj)
}
}
for _, obj := range structs {
name := obj.Name()
for iname, iface := range interfaces {
ptr := types.NewPointer(obj.Type())
if types.Implements(ptr, iface) {
interfaceToStruct[iname] = append(interfaceToStruct[iname], name)
}
}
}
b := bytes.NewBuffer(make([]byte, 0, 4096))
fmt.Fprintf(b, "package %s", modelsPkg.Name)
for iname, snames := range interfaceToStruct {
fmt.Fprintf(b, "\nfunc Map%sToName(obj %s) (string, error) {", iname, iname)
b.WriteString("\n\tswitch obj.(type) {")
for _, sname := range snames {
fmt.Fprintf(b, "\n\t\tcase *%s: return \"%s\", nil", sname, sname)
}
fmt.Fprintf(b, "\n\t\tdefault: return \"\", nil")
b.WriteString("\n\t}")
b.WriteString("\n}")
fmt.Fprintf(b, "\nfunc MapNameTo%s(name string) (%s, error) {", iname, iname)
b.WriteString("\n\tswitch name {")
for _, sname := range snames {
fmt.Fprintf(b, "\n\t\tcase \"%s\":", sname)
fmt.Fprintf(b, "\n\t\t\treturn new(%s), nil", sname)
}
fmt.Fprintf(b, "\n\t\tdefault: return nil, nil")
b.WriteString("\n\t}")
b.WriteString("\n}")
}
} |
I've been trying to find a neat way to deal with serialising a number of different types behind an interface. So far I've hit on two things that appear to work. One is to introduce an intermediate structure that has a 2-tuple of type/data, the other is to hack in a msgpack extension (which you explicitly recommend against in #35 - might be good to add that recommendation to the docs!).
In spite of the recommendation, the latter does indeed work, though not particularly well. It introduces a lot of intermediate copying of bytes and calculating
Len()
is difficult. I worked around that by marshalling inLen()
and caching the result for the subsequent marshal, but you know... ew.At the moment, I only have a few structures in my application that need this, but we are looking to serialise a heap more stuff so that's set to explode and a lot of the new stuff requires some sort of polymorhpism. I've been surviving by doing my own code generation of manually written msgp calls for the wrapper structs, but I'd really like a way to do this that doesn't require the intermediate struct because that will gunk up the application pretty badly.
I've been thinking something like another directive, like
//msgp:maptype ...
, which would inline [type, data] as a 2-length array in place of the value itself. I'm imagining it could look something like this:The mappers themselves are a bit annoying to write but it does seem to work well. I use ints for the kind but some may want to use strings.
At the moment, my janky hack version is producing Encoders that look like this:
If this (or something else that achieves the same goal) is something you'd consider, I'm quite happy to try to work up a PR that meets your requirements. It'd be great to see some support for this in msgp, it's a pretty common problem.
The text was updated successfully, but these errors were encountered: