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

Codegen various improvements #106

Merged
merged 7 commits into from
Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 29 additions & 20 deletions schema/gen/go/_demo/omg-schemas/schemaschema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,27 +132,27 @@ func init() {
ts.Accumulate(schema.SpawnUnion("MapRepresentation",
[]schema.TypeName{
"MapRepresentation_Map",
"MapRepresentation_StringPairs",
"MapRepresentation_ListPairs",
"MapRepresentation_Stringpairs",
"MapRepresentation_Listpairs",
},
schema.SpawnUnionRepresentationKeyed(map[string]schema.TypeName{
"map": "MapRepresentation_Map",
"stringpairs": "MapRepresentation_StringPairs",
"listpairs": "MapRepresentation_ListPairs",
"stringpairs": "MapRepresentation_Stringpairs",
"listpairs": "MapRepresentation_Listpairs",
}),
))
ts.Accumulate(schema.SpawnStruct("MapRepresentation_Map",
[]schema.StructField{},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("MapRepresentation_StringPairs",
ts.Accumulate(schema.SpawnStruct("MapRepresentation_Stringpairs",
[]schema.StructField{
schema.SpawnStructField("innerDelim", "String", false, false),
schema.SpawnStructField("entryDelim", "String", false, false),
},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("MapRepresentation_ListPairs",
ts.Accumulate(schema.SpawnStruct("MapRepresentation_Listpairs",
[]schema.StructField{},
schema.StructRepresentation_Map{},
))
Expand Down Expand Up @@ -193,14 +193,16 @@ func init() {
"UnionRepresentation_Keyed",
"UnionRepresentation_Envelope",
"UnionRepresentation_Inline",
"UnionRepresentation_StringPrefix",
"UnionRepresentation_BytePrefix",
},
schema.SpawnUnionRepresentationKeyed(map[string]schema.TypeName{
"kinded": "UnionRepresentation_Kinded",
"keyed": "UnionRepresentation_Keyed",
"envelope": "UnionRepresentation_Envelope",
"inline": "UnionRepresentation_Inline",
"byteprefix": "UnionRepresentation_BytePrefix",
"kinded": "UnionRepresentation_Kinded",
"keyed": "UnionRepresentation_Keyed",
"envelope": "UnionRepresentation_Envelope",
"inline": "UnionRepresentation_Inline",
"stringprefix": "UnionRepresentation_StringPrefix",
"byteprefix": "UnionRepresentation_BytePrefix",
}),
))
ts.Accumulate(schema.SpawnMap("UnionRepresentation_Kinded",
Expand All @@ -224,8 +226,15 @@ func init() {
},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("UnionRepresentation_StringPrefix",
[]schema.StructField{
schema.SpawnStructField("discriminantTable", "Map__String__TypeName", false, false), // todo: dodging inline defn's again.
},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("UnionRepresentation_BytePrefix",
[]schema.StructField{
// REVIEW: for schema-schema overall: this is a very funny type. Should we use strings here? And perhaps make it use hex for maximum clarity? This would also allow multi-byte prefixes, which would match what's already done by stringprefix representation.
schema.SpawnStructField("discriminantTable", "Map__TypeName__Int", false, false), // todo: dodging inline defn's again.
},
schema.StructRepresentation_Map{},
Expand Down Expand Up @@ -260,16 +269,16 @@ func init() {
[]schema.TypeName{
"StructRepresentation_Map",
"StructRepresentation_Tuple",
"StructRepresentation_StringPairs",
"StructRepresentation_StringJoin",
"StructRepresentation_ListPairs",
"StructRepresentation_Stringpairs",
"StructRepresentation_Stringjoin",
"StructRepresentation_Listpairs",
},
schema.SpawnUnionRepresentationKeyed(map[string]schema.TypeName{
"map": "StructRepresentation_Map",
"tuple": "StructRepresentation_Tuple",
"stringpairs": "StructRepresentation_StringPairs",
"stringjoin": "StructRepresentation_StringJoin",
"listpairs": "StructRepresentation_ListPairs",
"stringpairs": "StructRepresentation_Stringpairs",
"stringjoin": "StructRepresentation_Stringjoin",
"listpairs": "StructRepresentation_Listpairs",
}),
))
ts.Accumulate(schema.SpawnStruct("StructRepresentation_Map",
Expand Down Expand Up @@ -297,21 +306,21 @@ func init() {
ts.Accumulate(schema.SpawnList("List__FieldName",
"FieldName", false,
))
ts.Accumulate(schema.SpawnStruct("StructRepresentation_StringPairs",
ts.Accumulate(schema.SpawnStruct("StructRepresentation_Stringpairs",
[]schema.StructField{
schema.SpawnStructField("innerDelim", "String", false, false),
schema.SpawnStructField("entryDelim", "String", false, false),
},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("StructRepresentation_StringJoin",
ts.Accumulate(schema.SpawnStruct("StructRepresentation_Stringjoin",
[]schema.StructField{
schema.SpawnStructField("join", "String", false, false), // review: "delim" would seem more consistent with others -- but this is currently what the schema-schema says.
schema.SpawnStructField("fieldOrder", "List__FieldName", true, false), // todo: dodging inline defn's again.
},
schema.StructRepresentation_Map{},
))
ts.Accumulate(schema.SpawnStruct("StructRepresentation_ListPairs",
ts.Accumulate(schema.SpawnStruct("StructRepresentation_Listpairs",
[]schema.StructField{},
schema.StructRepresentation_Map{},
))
Expand Down
69 changes: 68 additions & 1 deletion schema/gen/go/genList.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,74 @@ func (g listGenerator) EmitNativeType(w io.Writer) {
}

func (g listGenerator) EmitNativeAccessors(w io.Writer) {
// FUTURE: come back to this -- surely something nice can be done here.
// Generate a speciated Lookup as well as LookupMaybe method.
// The Lookup method returns nil in case of *either* an out-of-range/absent value or a null value,
// and so should only be used if the list type doesn't allow nullable keys or if the caller doesn't care about the difference.
// The LookupMaybe method returns a MaybeT type for the list value,
// and is needed if the list allows nullable values and the caller wishes to distinguish between null and out-of-range/absent.
// (The Lookup method should be preferred for lists that have non-nullable keys, because LookupMaybe may incur additional costs;
// boxing something into a maybe when it wasn't already stored that way costs an alloc(!),
// and may additionally incur a memcpy if the maybe for the value type doesn't use pointers internally).
doTemplate(`
func (n *_{{ .Type | TypeSymbol }}) Lookup(idx int) {{ .Type.ValueType | TypeSymbol }} {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought on naming: i might more naturally think of this at Get rather than Lookup

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this for a surprisingly long time. At this point, I'd say either all the methods on the Node interface turn into Get* and this goes with them, or, none of them do. Having the gen'd methods with concrete types have a totally different name prefix than the generic interface methods was the one thing that was definitely a mistake.

if n.Length() <= idx {
return nil
}
v := &n.x[idx]
{{- if .Type.ValueIsNullable }}
if v.m == schema.Maybe_Null {
return nil
}
return {{ if not (MaybeUsesPtr .Type.ValueType) }}&{{end}}v.v
{{- else}}
return v
{{- end}}
}
func (n *_{{ .Type | TypeSymbol }}) LookupMaybe(idx int) Maybe{{ .Type.ValueType | TypeSymbol }} {
if n.Length() <= idx {
return nil
}
v := &n.x[idx]
{{- if .Type.ValueIsNullable }}
return v
{{- else}}
return &_{{ .Type.ValueType | TypeSymbol }}__Maybe{
m: schema.Maybe_Value,
v: {{ if not (MaybeUsesPtr .Type.ValueType) }}*{{end}}v,
}
{{- end}}
}

var _{{ .Type | TypeSymbol }}__valueAbsent = _{{ .Type.ValueType | TypeSymbol }}__Maybe{m:schema.Maybe_Absent}
`, w, g.AdjCfg, g)

// Generate a speciated iterator.
// The main advantage of this over the general ipld.ListIterator is of course keeping types visible (and concrete, to the compiler's eyes in optimizations, too).
// It also elides the error return from the iterator's Next method. (Overreads will result in -1 as an index and nil values; this is both easily avoidable, and unambiguous if you do goof and hit it.)
doTemplate(`
func (n {{ .Type | TypeSymbol }}) Iterator() *{{ .Type | TypeSymbol }}__Itr {
return &{{ .Type | TypeSymbol }}__Itr{n, 0}
}

type {{ .Type | TypeSymbol }}__Itr struct {
n {{ .Type | TypeSymbol }}
idx int
}

func (itr *{{ .Type | TypeSymbol }}__Itr) Next() (idx int, v {{if .Type.ValueIsNullable }}Maybe{{end}}{{ .Type.ValueType | TypeSymbol }}) {
if itr.idx >= len(itr.n.x) {
return -1, nil
}
idx = itr.idx
v = &itr.n.x[itr.idx]
itr.idx++
return
}
func (itr *{{ .Type | TypeSymbol }}__Itr) Done() bool {
return itr.idx >= len(itr.n.x)
}

`, w, g.AdjCfg, g)
}

func (g listGenerator) EmitNativeBuilder(w io.Writer) {
Expand Down
63 changes: 51 additions & 12 deletions schema/gen/go/genMap.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,31 @@ func (g mapGenerator) EmitNativeType(w io.Writer) {

func (g mapGenerator) EmitNativeAccessors(w io.Writer) {
// Generate a speciated Lookup as well as LookupMaybe method.
// The LookupMaybe method is needed if the map value is nullable and you're going to distinguish nulls
// (and may also be convenient if you would rather handle Maybe_Absent than an error for not-found).
// The Lookup method works fine if the map value isn't nullable
// (and should be preferred in that case, because boxing something into a maybe when it wasn't already stored that way costs an alloc(!),
// and may additionally incur a memcpy if the maybe for the value type doesn't use pointers internally).
// REVIEW: is there a way we can make this less twisty? it is VERY unfortunate if the user has to know what sort of map it is to know which method to prefer.
// Maybe the Lookup method on maps that have nullable values should just always have a MaybeT return type?
// But then this means the Lookup method doesn't "need" an error as part of its return signiture, which just shuffles differences around.
// The Lookup method returns nil in case of *either* an absent value or a null value,
// and so should only be used if the map type doesn't allow nullable keys or if the caller doesn't care about the difference.
// The LookupMaybe method returns a MaybeT type for the map value,
// and is needed if the map allows nullable values and the caller wishes to distinguish between null and absent.
// (The Lookup method should be preferred for maps that have non-nullable keys, because LookupMaybe may incur additional costs;
// boxing something into a maybe when it wasn't already stored that way costs an alloc(!),
// and may additionally incur a memcpy if the maybe for the value type doesn't use pointers internally).
doTemplate(`
func (n *_{{ .Type | TypeSymbol }}) Lookup(k {{ .Type.KeyType | TypeSymbol }}) {{ .Type.ValueType | TypeSymbol }} {
v, exists := n.m[*k]
if !exists {
return nil
}
{{- if .Type.ValueIsNullable }}
if v.m == schema.Maybe_Null {
return nil
}
return {{ if not (MaybeUsesPtr .Type.ValueType) }}&{{end}}v.v
{{- else}}
return v
{{- end}}
}
func (n *_{{ .Type | TypeSymbol }}) LookupMaybe(k {{ .Type.KeyType | TypeSymbol }}) Maybe{{ .Type.ValueType | TypeSymbol }} {
v, ok := n.m[*k]
if !ok {
v, exists := n.m[*k]
if !exists {
return &_{{ .Type | TypeSymbol }}__valueAbsent
}
{{- if .Type.ValueIsNullable }}
Expand All @@ -70,10 +83,36 @@ func (g mapGenerator) EmitNativeAccessors(w io.Writer) {
}

var _{{ .Type | TypeSymbol }}__valueAbsent = _{{ .Type.ValueType | TypeSymbol }}__Maybe{m:schema.Maybe_Absent}
`, w, g.AdjCfg, g)

// Generate a speciated iterator.
// The main advantage of this over the general ipld.MapIterator is of course keeping types visible (and concrete, to the compiler's eyes in optimizations, too).
// It also elides the error return from the iterator's Next method. (Overreads will result in nil keys; this is both easily avoidable, and unambiguous if you do goof and hit it.)
doTemplate(`
func (n {{ .Type | TypeSymbol }}) Iterator() *{{ .Type | TypeSymbol }}__Itr {
return &{{ .Type | TypeSymbol }}__Itr{n, 0}
}

type {{ .Type | TypeSymbol }}__Itr struct {
n {{ .Type | TypeSymbol }}
idx int
}

func (itr *{{ .Type | TypeSymbol }}__Itr) Next() (k {{ .Type.KeyType | TypeSymbol }}, v {{if .Type.ValueIsNullable }}Maybe{{end}}{{ .Type.ValueType | TypeSymbol }}) {
if itr.idx >= len(itr.n.t) {
return nil, nil
}
x := &itr.n.t[itr.idx]
k = &x.k
v = &x.v
itr.idx++
return
}
func (itr *{{ .Type | TypeSymbol }}__Itr) Done() bool {
return itr.idx >= len(itr.n.t)
}

// TODO generate also a plain Lookup method that doesn't box and alloc if this type contains non-nullable values!
`, w, g.AdjCfg, g)
// FUTURE: also a speciated iterator?
}

func (g mapGenerator) EmitNativeBuilder(w io.Writer) {
Expand Down
2 changes: 1 addition & 1 deletion schema/gen/go/genStruct.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (g structGenerator) EmitNativeAccessors(w io.Writer) {
doTemplate(`
{{- $type := .Type -}} {{- /* ranging modifies dot, unhelpfully */ -}}
{{- range $field := .Type.Fields }}
func (n _{{ $type | TypeSymbol }}) Field{{ $field | FieldSymbolUpper }}() {{ if $field.IsMaybe }}Maybe{{end}}{{ $field.Type | TypeSymbol }} {
func (n _{{ $type | TypeSymbol }}) Field{{ $field | FieldSymbolUpper }}() {{ if $field.IsMaybe }}Maybe{{end}}{{ $field.Type | TypeSymbol }} {
return &n.{{ $field | FieldSymbolLower }}
}
{{- end}}
Expand Down