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

Kinded union gen #64

Merged
merged 7 commits into from
Aug 19, 2020
Merged

Kinded union gen #64

merged 7 commits into from
Aug 19, 2020

Conversation

warpfork
Copy link
Collaborator

@warpfork warpfork commented Aug 19, 2020

Kinded unions are now generateable!

My favorite part of the diff is quite simply the feature table change:

-| ... kinded representation      |     ✘     |     ✘    |
+| ... kinded representation      |     ✔     |     ✔    |

Kinded unions are pretty neat to have, because between them and keyed unions, we can do most of the stuff that's Interesting. (Substituting keyed unions for other strategies as a placeholder during prototyping systems is generally fine (in addition to the fact we simply use them the most in new designs anyway); kinded unions less so, because they change the topology of the document rather noticeably.) I've started prototyping a bunch of stuff using them.


It has turned out we can also now generate any "Any" type using kinded unions and really no additional special effort at all, which is pretty neato:

	ts.Accumulate(schema.SpawnUnion("Any",
		[]schema.TypeName{
			"Bool",
			"Int",
			"Float",
			"String",
			"Bytes",
			"Map",
			"List",
			"Link",
		},
		schema.SpawnUnionRepresentationKinded(map[ipld.ReprKind]schema.TypeName{
			ipld.ReprKind_Bool:   "Bool",
			ipld.ReprKind_Int:    "Int",
			ipld.ReprKind_Float:  "Float",
			ipld.ReprKind_String: "String",
			ipld.ReprKind_Bytes:  "Bytes",
			ipld.ReprKind_Map:    "Map",
			ipld.ReprKind_List:   "List",
			ipld.ReprKind_Link:   "Link",
		}),
	))

... will result in codegen of a perfectly reasonable compact little structure:

type _Any struct {
	tag uint
	x1 _Bool
	x2 _Int
	x3 _Float
	x4 _String
	x5 _Bytes
	x6 _Map
	x7 _List
	x8 _Link
}

Something like this will probably end up in the "prelude" of default types in a schema.

(The types named Map and List here are type Map {String:nullable Any} and type List [nullable Any], which is a fun cycle, but turns out to fly right, and still leaves us with a self-contained "prelude".)

Alternately, if the adjunct config contains CfgUnionMemlayout: map[schema.TypeName]string{"Any": "interface"},, then you'll get:

type _Any struct {
	x _Any__iface
}
type _Any__iface interface {
	_Any__member()
}
func (_Bool) _Any__member()   {}
func (_Int) _Any__member()    {}
func (_Float) _Any__member()  {}
func (_String) _Any__member() {}
func (_Bytes) _Any__member()  {}
func (_Map) _Any__member()    {}
func (_List) _Any__member()   {}
func (_Link) _Any__member()   {}

Implementing this was a bit more of a bear than other things: the representation types for kinded unions don't get to use any of the other helpers and mixins we've made for all the other systems, because they don't consistently behave like any single particular kind! So, lots of stuff that would've been taken care of by one of the mixins in any of the other representations we've coded up so far end up being a bit more manual and unique here.

The schema.Type interface also sprouted a new method: RepresentationBehavior(), which says what a type's representation mode acts like. It's necessary for the codegen code to look at this information on the member types of a kinded union when making some choices for its internals.

I used a different kind of code reuse in the course of this: textual concatenation of template segments. Seemed like as good a time as any to try it out as a style, since the mixins pattern was inapplicable anyway. It... well, it works. I don't think it turned out particularly well, though. I'm okay enough with it to merge it for now, but in the future it might be preferable to avoid this style of writing; or at the very least, make the parameters themselves be something with a more logical understanding of things, rather than being template snippets. (The number of template snippets needed to complete the task grew a lot faster than I expected; you can see the progress from "this is fine" to "ugh" in the procession of commits in the PR if you're interested in the evolutionary record.)


A new document of HACKME_dry.md, with remarks on the various mechanisms for "don't repeat yourself" used in the codegen system, has appeared, incidentally.


There are a few bugs I've found as I've tried to exercise this in downstream prototypes. However, I don't think they're due to the code that's new in this PR (and I'm just finding them because I'm doing larger and larger prototype usage); so, I'm going to merge this, and then chase reproducers and tests for those bugs, and then their fixes, in separate branches subsequently to keep things readable.

So far, the generation itself runs, but the result will not yet compile.
I just want a checkpoint here.  (Most of this was written a few days ago already.)
…ors.

This template munging business got nastier as time proceded.
I think we could drastically reduce the amount of redundancy in the
arugments to `kindedUnionNodeMethodTemplateMunge` if we re-did that
function with a more structural understanding of what's going on,
and made some basic understanding of what zero values are per golang
type, etc.

That can be future work, though.  Today, I want working kinded unions,
and I just want them done enough to use; kludge tolerance high.
The AssignNode method is now supplied; and the previous hacky shrug
regarding switch statements with no applicable cases also had to go.
(Turns out that the no-applicable-cases thing coincidentally worked
for the "embedall" internal implementation mode; but didn't fly for
the "interface" internal style, because that would end up with
variable declarations made but not referenced.  Harrumph.)
@warpfork warpfork merged commit d3ccd3c into master Aug 19, 2020
@warpfork warpfork deleted the kinded-union-gen branch August 19, 2020 14:40
@rvagg
Copy link
Member

rvagg commented Aug 19, 2020

"textual concatenation of template strings" - i.e. the *TemplateMunge() methods? Gives you DRY here but not the generated code which is still going to be quite large, right?

Re "behavior", it seems to me that this is just a synonym for "representation kind". Is it only introduced here because there's no way to publicly access the representation property of the various types? Because it could just be a property of each of the representations, right? I think I'm just concerned about introducing new terminology here that has (near?) 100% overlap with what we already have.

@warpfork
Copy link
Collaborator Author

warpfork commented Aug 20, 2020

Yeah, some sort of method is needed on the interface because I didn't want to destructure my way into each RepresentationStrategy -- mind, there's no single RepresentationStrategy interface at all; it's just a convention of things -- and attach the property there. There's no way to get a RepresentationStrategy from a Type in general.

I guess adding a RepresentationStrategy interface would've also been a valid way through this. It didn't really occur to me, because this is the first time that we've needed the same property from each one of them. So far, everything else has required destructuring because things have been full of unique behavior per strategy. I'd be open to revisiting this.

(Type).RepresentationBehavior() could've also been called (Type).RepresentationKind(), I suppose, yes. (Minding, however, that it can be slightly different than somevalue.Representation().Kind() -- kinded unions have to say "invalid" when asked on the type info alone, because it's not known, whereas asking it on a value can return a valid answer, based on the actual inhabitant.)

@rvagg
Copy link
Member

rvagg commented Aug 21, 2020

good thing you already have ipld.ReprKind_Invalid then

@warpfork
Copy link
Collaborator Author

Yeah... I feel kind of sketched out by that constant, honestly; there's times where it's a valid sentinel (okay, one time: this, here), and many times where it's not (and I just marked it explicitly so that the appearance of 'zero' value clearly means an uninitialized structure was found and that it's a bug). But doing something safer in golang would require about a hundred lines more boilerplate than currently seems justifiable.

@aschmahmann aschmahmann mentioned this pull request Feb 18, 2021
73 tasks
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

Successfully merging this pull request may close these issues.

2 participants