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

gengo: support for unions with stringprefix representation. #133

Merged
merged 2 commits into from
Jan 7, 2021
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
7 changes: 7 additions & 0 deletions node/mixins/delim.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,10 @@ func SplitExact(s string, sep string, count int) ([]string, error) {
}
return ss, nil
}

// SplitN is an alias of strings.SplitN, which is only present here to
// make it usable in codegen packages without requiring conditional imports
// in the generation process.
func SplitN(s, sep string, n int) []string {
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, just run goimports on the generated code? "figure out what imports we need" is a solved problem in go :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That seems significantly more complex. I'm not okay with pushing a need for goimports -- something from the x/tools repos -- onto users in order to have compilable code come out. (By contrast, while elsewhere we decided we're okay leaning on post-gen invocation of gofmt, that differs on two fronts: it's effectively always available, and it's also not strictly necessary to apply before compile will succeed.)

A small handful of cherry-picked constants like this seems fine to me.

We may want to consider moving such things to their own small package (or the minima file in the output?) to make them less obtrusive, though.

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 guess goimports is probably also possible to use as a library, so we could just add it to our own dependencies. But I'd still say for this small of a need, it's probably not worth it. I'd favor KISS on this until we really have a desire for conditional imports.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the assumption we'd be the only ones running this generator. Why would other people generate their own mixins directly?

Copy link
Collaborator Author

@warpfork warpfork Jan 9, 2021

Choose a reason for hiding this comment

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

Maybe the comment is unclear -- SplitN is used in the parsing logic of the code that comes out.

https://github.com/ipld/go-ipld-prime/pull/133/files#diff-7b9d4887e3f1a6fe25da2c76ec88a55a2e71169f757cbe9b5c1cf2352f1985c3R155

return strings.SplitN(s, sep, n)
}
135 changes: 68 additions & 67 deletions schema/gen/go/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,70 +47,71 @@ Legend:
- `?` - feature definition needed! (applies to many of the "native extras" rows -- often there's partial features, but also room for more.)
- ` ` - table is not finished, please refer to the code and help fix the table :)

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| structs | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... map representation | ✔ | ✔ |
| ... ... including optional | ✔ | ✔ |
| ... ... including renames | ✔ | ✔ |
| ... ... including implicits | ⚠ | ⚠ |
| ... tuple representation | ✔ | ✔ |
| ... ... including optional | ✔ | ✔ |
| ... ... including renames | - | - |
| ... ... including implicits | ⚠ | ⚠ |
| ... stringjoin representation | ✔ | ✔ |
| ... ... including optional | - | - |
| ... ... including renames | - | - |
| ... ... including implicits | - | - |
| ... stringpairs representation | ✘ | ✘ |
| ... ... including optional | | |
| ... ... including renames | | |
| ... ... including implicits | | |
| ... listpairs representation | ✘ | ✘ |
| ... ... including optional | | |
| ... ... including renames | | |
| ... ... including implicits | | |

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| lists | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... list representation | ✔ | ✔ |

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| maps | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... map representation | ✔ | ✔ |
| ... stringpairs representation | ✘ | ✘ |
| ... listpairs representation | ✘ | ✘ |

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| unions | ... | ... |
| ... type level | ✔ | ✔ |
| ... keyed representation | ✔ | ✔ |
| ... envelope representation | ✘ | ✘ |
| ... kinded representation | ✔ | ✔ |
| ... inline representation | ✘ | ✘ |
| ... byteprefix representation | ✘ | ✘ |

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| strings | ✔ | ✔ |
| bytes | ✔ | ✔ |
| ints | ✔ | ✔ |
| floats | ✔ | ✔ |
| bools | ✔ | ✔ |
| links | ✔ | ✔ |

| feature | accessors | builders |
|:-------------------------------|:---------:|:--------:|
| enums | ... | ... |
| ... type level | ✘ | ✘ |
| ... string representation | ✘ | ✘ |
| ... int representation | ✘ | ✘ |
| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| structs | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... map representation | ✔ | ✔ |
| ... ... including optional | ✔ | ✔ |
| ... ... including renames | ✔ | ✔ |
| ... ... including implicits | ⚠ | ⚠ |
| ... tuple representation | ✔ | ✔ |
| ... ... including optional | ✔ | ✔ |
| ... ... including renames | - | - |
| ... ... including implicits | ⚠ | ⚠ |
| ... stringjoin representation | ✔ | ✔ |
| ... ... including optional | - | - |
| ... ... including renames | - | - |
| ... ... including implicits | - | - |
| ... stringpairs representation | ✘ | ✘ |
| ... ... including optional | | |
| ... ... including renames | | |
| ... ... including implicits | | |
| ... listpairs representation | ✘ | ✘ |
| ... ... including optional | | |
| ... ... including renames | | |
| ... ... including implicits | | |

| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| lists | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... list representation | ✔ | ✔ |

| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| maps | ... | ... |
| ... type level | ✔ | ✔ |
| ... native extras | ? | ? |
| ... map representation | ✔ | ✔ |
| ... stringpairs representation | ✘ | ✘ |
| ... listpairs representation | ✘ | ✘ |

| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| unions | ... | ... |
| ... type level | ✔ | ✔ |
| ... keyed representation | ✔ | ✔ |
| ... envelope representation | ✘ | ✘ |
| ... kinded representation | ✔ | ✔ |
| ... inline representation | ✘ | ✘ |
| ... stringprefix representation | ✔ | ✔ |
| ... byteprefix representation | ✘ | ✘ |

| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| strings | ✔ | ✔ |
| bytes | ✔ | ✔ |
| ints | ✔ | ✔ |
| floats | ✔ | ✔ |
| bools | ✔ | ✔ |
| links | ✔ | ✔ |

| feature | accessors | builders |
|:---------------------------------|:---------:|:--------:|
| enums | ... | ... |
| ... type level | ✘ | ✘ |
| ... string representation | ✘ | ✘ |
| ... int representation | ✘ | ✘ |
9 changes: 8 additions & 1 deletion schema/gen/go/genUnion.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,17 @@ func (g unionGenerator) EmitNativeType(w io.Writer) {
//
// The interface *mostly* isn't used... except for in the return type of a speciated function which can be used to do golang-native type switches.
//
// The interface also includes a requirement for an errorless primitive access method (such as `String() string`)
// if our representation strategy is one that has that semantic (e.g., stringprefix repr does).
//
// A note about index: in all cases the index of a member type is used, we increment it by one, to avoid using zero.
// We do this because it's desirable to reserve the zero in the 'tag' field (if we generate one) as a sentinel value
// (see further comments in the EmitNodeAssemblerType function);
// and since we do it in that one case, it's just as well to do it uniformly.
doTemplate(`
{{- if Comments -}}
// {{ .Type | TypeSymbol }} matches the IPLD Schema type "{{ .Type.Name }}". It has {{ .Type.TypeKind }} type-kind, and may be interrogated like {{ .Kind }} kind.
// {{ .Type | TypeSymbol }} matches the IPLD Schema type "{{ .Type.Name }}".
// {{ .Type | TypeSymbol }} has {{ .Type.TypeKind }} typekind, which means its data model behaviors are that of a {{ .Kind }} kind.
{{- end}}
type {{ .Type | TypeSymbol }} = *_{{ .Type | TypeSymbol }}
type _{{ .Type | TypeSymbol }} struct {
Expand All @@ -61,6 +65,9 @@ func (g unionGenerator) EmitNativeType(w io.Writer) {
}
type _{{ .Type | TypeSymbol }}__iface interface {
_{{ .Type | TypeSymbol }}__member()
{{- if (eq (.Type.RepresentationStrategy | printf "%T") "schema.UnionRepresentation_Stringprefix") }}
String() string
{{- end}}
}

{{- range $member := .Type.Members }}
Expand Down
Loading