GitLab

This first crossed my mind when considering whether it might be a good
idea to export the concrete implementation types of the 'basicnode'
package (see previous couple of commits -- it mostly had to do with the
question of whether we could reuse more code and shrink the size of
packages emitted by codegen)... but even though that's resolved to a
pretty solid "nope", it still makes sense to have a consistent name
prefix pattern here... and we'll probably encourage and expect to see
similar conventions in other packages (including in codegen, if we
add a handy-constructor-funcs feature to that).

And with this, the gendemo package both compiles and passes tests
once again.  yay!  Onward!

Check out the 'HACKME_scalars.md' file for why this commit represents
a bunch of nontrivial decisions.  There are a lot of possible ways
the compile failures here could be fixed, but some of them would have
bigger consequences of longterm weirdness than others.

Or at least, it certainly looks that way from here.  Maybe we'll see.

The scalar implementations are almost exact replicates of the basicnode
implementations.  Oddly, they haven't been exported yet.  This may
change (again, see discussion in the HACKME_scalars document) -- in
which case the code will also change to use wrapper structs.
Note one other divergence: the error messages have different content
for the typename: we mention the package, since this will probably be
typical and useful for codegen-created types in general.

The gendemo package almost compiles again.  The rest of the fixes
aren't related to this topic, so come in the next commit.

Turns out there was a lot of tech debt already accumulating very very
rapidly due to the earlier whimiscal choice to lump the new basicnode
implementations and the demo-codegen implementations into the same
packages as benchmarking and research on this branch began.

The basicnode package is now extracted cleanly, and exists in the
'node/basic' path.  This 'basicnode' package is the equivalent of what
was formerly called 'ipldfree' on the master branch.

The demo codegen content is now under 'node/gendemo'.
Note that it is BROKEN in this commit; there was too much there to
clean up and address in one commit, and this one has already become
quite large and includes a bewildering number of moves as well as
finer content rearrangements.  Fixes will come in next commit(s).

(Why?  Well, the demo codegen content leaned on the basicnode types
for string and int.  Turns out this actually opens *quite* the
can of worms when you try to rectify it.
At the surface level, the basicnode package's types aren't exported.
Could we export them?  Well, sure.  Except that's only half the trick.
Can we export the *assemblers* for those types?  Well, sure.  But...
Can we export the 'w' fields for those assemblers for those types?
**NO** -- this is where it all breaks down -- if those fields are
exported, we lose all control over all immutability everywhere.
Shoot...?  Very shoot.
One clearly correct solution to this is to generate new types for
all of the primitive scalars in every codegen output package;
the downside, obviously, is that's a bit verbose in resulting code.
There might be another very alexandrian solution to this gordian knot:
exporting the basicnode package's types after all, but not their
assemblers; then leaning further into their implementation detail
of being implemented as typedefs rather than strictly enclosed structs,
and having every part of codegen types that handle primitive leafs
directly perform and manage the relevant casts.  The bigger picture
wisdom of this has not been completely analyzed yet.)

The 'impls' path has in general become 'node'.
This will stay this way when we finally merge over core on master;
all the current 'impl' (without the s; sigh) paths will become 'node'.

Numerous new HACKME files have appeared; some are splits of old ones
to account for the new package splits; some are hoisted and polished
from what were previously commentstorms in the code; some are
completely new.  There are also a few new package-scope godocs.

The benchmarks and tests are rearranged and much (much) improved.
Naming is more consistent; and things are consistently extracted
to the 'node/mixins/tests' package, and imported and used from there.
(If you want all the same benchmark info as before, you'll now have
to run benchmarks from several different packages.)

The last line is still particularly interesting and nonobvious, though.
It's good that the current design does *not* expose memory anywhere
during mid-construction, even to parts that are semi-finalized:
if we did, and it so happened that a backing array had to grow, we'd
end up releasing pointers to several backing arrays into the wild,
which would probably end up rather terrible and rather hard to see.

It's become clear that `map[string]struct{k, v Node}` is the correct
choice in a wide variety of ways; we've already gone all in on that.

The rest of the comment is speculative, and not wrong, but it'll be
just as well to reboot those speculations later, and many of the things
that were in flux when that comment was penned aren't anymore, so a
revisit should be simpler.

This is a rather important detail to remember, and one that is easy to
lose track of if not actively wrangling the schema typed usecases.
(And so, I almost did.  Uufdah.)

The various methods for getting NodeStyle out of assemblers are now
much better documented.

As my current goals are still revolving around finishing the new
'ipldfree'/'basic' nodes, wrapping up this whole research branch,
and bringing these interface back to land in the library core...
I'm still not bothering to substantially handle the inside of the
ValueStyle methods on the example struct types.  But it should be
clear enough how to implement them correctly now.

Previous description -- which stated the style of a node may vary based
on how it was created -- was outright wrong, and specified things both
difficult to implement and fairly useless.

There are still some less than completely written parts of the story
for generic transformations, but this definition is at least closer.

What it does is simple: an 'anyBuilder' will either delegate to a map
builder, or a list builder, or do a scalar build (directly, because
there's not enough to do there to justify using any indirection), *OR*
even simply hang onto and pass through another Node.

The tests ensure that it can be used transparently to pass all the
string tests (yay) as well as all the map tests (yayy!) including those
that exercise recursion (yayyy!!).  In addition to demonstrating that
the 'any' code works correctly, it's also a nice demostration of
reusable tests and behavioral specifications paying off.  (There's more
work to do here to clean them up, standardize naming, etc, but it's
a nice start.)

The 'anyInhabitedBy{Kind}' stuff stamped out at the bottom is... well,
committing it for posterity, but in fact I'm going to remove that in
the very next commit, as well as some docs on the subject from a few
commits ago that are now showing as incorrect: the idea of the style
of a node varying based on how it was produced is really... *cough*
I'm not sure how I got that in my head; it clearly doesn't fly right.

Also map able to create child list.

Almost the same as the old ones.  Builders and Styles change, ofc.

Also took the liberty of adding TypeName values to all ErrWrongKind.
It's apparently now the convention that untyped things set that with
the lowercase kind name.

This... just about completes the pantheon of basic stuff in ipldfree
that's getting a reimplementation.

Maps need to be updated to also support recursing into a list, still.

Tests needed badly.  Next couple of commits will start handling that.
Need to reorganize some of the existing ones as well.  This is clearly
going to be one of those things that needs a large corpus... and for
bonus fun, we need to make a corpus that's ready to go for both working
with schemas and without.  Fun fun fun.

For all the paths around scalar children, invalidating the child
assembler's pointer back to parent has been sufficient to obstruct any
problematic mutations, but with how directly the operations on map
children are passed through to their 'w' target, we also need to do
an invalidation that affects there.

Interestingly, the codegen'd map ends up doing this in a different way
than the generic map: since it *is* actually wrapping and delegating to
another assembler to handle the child value, it has to make sure *that*
is invalidated; otherwise the fact we hand calls to the child assembler
right away before doing our 'flush' would be problematic.

Splitting the 'flush' in half would also work, of course.  At present
it seems like six of one and half a dozen of the other.  We can subject
this to microbenchmarking later if it seems relevant.

And tests for them and when they should fail.

Which provokes the inclusion of a few fixes, to make things fail where
they dang well should.

Tests, yo.  They're definitely important.  *Especially* for when things
get *off* the happy path.

And some misc fixes in others (inconsistent method ordering where I
updated existing files, mostly).

Lists still coming up; being a recursive kind, those have much more
involved code.

(Probably should've done this a while ago.  Much earlier in this
research branch, I thought the design of Link/LinkBuilder etc might
get a review... but at this point, its clear there's plenty of
work already do to just sorting out the assembler situation, so
any rethinks of links is *definitely* deferred for future investigation
(if indeed anything ever happens there at all).)

Doing very partial fixes to the example "generated" maps;
just enough to compile.  I'm not intending to use them as a template
for actual codegen later, so their quality is irrelevant at this point.

To be fair, the compiler would've caught this if I didn't still have
a dummy placeholder definition of 'Link' in the base interface package.

Putting it back in the NodeBuilder interface in the next commit;
that also needs all the impl fixes which might be a tad longer.
Signed-off-by: Eric Myhre <hash@exultant.us>

It's clearly a string in all behavioral ways.

I do pause to wonder if a different "TypeName" should be used here to
provide more indication of context.  I don't have a strong argument
one way or another.  Leaning against it by default, since so far when
I've had questions about "should I make child assemblers announce
their specialness" it has usually seemed to shake out to "no".

This removes all the shitty placeholder `panic("no")` calls which
previously occupied this area, and gets us one step closer to being at
the quality level where we can merge this into core.

Also, a short template file to make it easier to stamp out the rest of
these for the remaining kinds.  No automation; not worth it.

As has been the priority in other cases, the shorter name should be
reserved for codegen outputs, as those are where we are optimizing for
ergonomics the most.

They don't reduce line count, but they do make lines much shorter,
and increase consistency, which are worth it.

There's some very similar things already in the codegen system,
though under a different name ("kindedRejectionHelper" something).
I've realized the utility also exists beyond codegen.

Codegen can in the future be updated to use these (and in the process,
emit noticably fewer bytes of generated code).

(There could be concerns about codegen output getting overly fragile
and dependent on core library versions by reusing more code like this.
However, I don't think that applies here: It's already going to be
fragile if any of the actual error types change; being fragile if the
mixins (which are *almost* entirely about those) change is effectively
the same thing, it just saves a lot of output size.)

I would collapse all these uninteresting methods into one line each
rather than three lines each if I could... however, the golang
formatter's (rather incogruous?) rule about breaking long lines in
this *particular* case strikes.  Turns out in this corpus, for example,
the `plainInt.LookupSegment` declaration *just* crosses over the
length which forces a break, even though none of its siblings do.
Sigh.

The previously specified behavior has turned out to be *remarkably*
annoying; much more so than I initially expected.  It's both not
particularly pleasant to handle as a user; and even more unexpectedly,
has turned out absolutely infuriating as a library author.
(You can see how the early work on these new packages has simply
resorted to a placeholder panic there, because the reject-carrying
thunks cost an irritating amount of unimportant work.)

The mapIteratorReject and listIteratorReject thunks have replicated
too many times already, and this count is indicative of a problem:
I'm not replicating them again into this new generation of interfaces.
Nope.  Nuked.

This prevents later mutation by holding onto an assembler too long.
(*Getting* the child assemblers is fenced by the 'state' field;
but since none of the child assembler methods check the 'state',
some defense is needed there.  Invalidating the pointer back up is it:
this invalidation overhold of child assemblers once the user already
has them into a nonissue by making any mutations fail.)

I did a few extra-long benchmark runs to make sure this extra footwork
doesn't cost any noticable time.  It does not; it's within the margins
of error on a benchtime=4m run.

We use the word "done" as the predicate in iterators; therefore it's
confusing to also use it as a verb for *doing* completion elsewhere.

"Finish" also has the advantage of sounding slightly more active --
which it is, since it's often involved in housekeeping and, well,
*finishing* assignment within the enclosing parent value, as well as
in evaluating validations for types that have them.

The original goal of insuring there's minimal speed costs turned out to
be a red herring because it was already doing pretty reasonable things;
but instead, the quest generated a little learning about assembly size.

Comments about that continue to accumulate in the
currently-backburner'd map+structs codegen file, where they are most
relevant because of the question of whether to handle struct fields
with generation of copious small types.

In the assembly, this ends up inlined, so there's no reduction in size,
but there's also no reduction in speed.

Can you believe this required another wrapper type?  But indeed:
this is the case: when building a recursive value, it is necessary for
us to do some actions after that recursed operation becomes 'done'.
In this case, we have to both kick the parent state machine along, and
also finish the actual assignment into the map!  (In codegen maps,
the latter need isn't true, for fun pointer dance reasons, but the
need for state machine kicking -- as well as having the error checking
branch to make sure any validation succeeded! -- is still applicable.)

Tests all pass.

When I started typing this, I thought I was going to need it -- or at
least *want* it -- in the recursive bits of map value assemblers.
By the end, it turns out... no, actually, I don't.  Map value assembler
does better just embedding the (very sparse) amount of relevant logic
without abstraction in this case; and it also *couldn't* embed the
anyBuilder regardless, because the anyBuilder embeds the map assembler
which embeds it's value assembler..... Fhwooo.

As you can see in some of the comments, I was also surprised to find
that it currently shakes out that there's no demand at all for an
'anyNode' type.  There could be! -- if we had maps and lists use that
rather space-costly mechanism in exchange for fewer value allocs -- but
at present, we don't.

But even if that supposed dependency path turned out wrong, of course
we still need an 'anyBuilder' regardless -- deserialization often
starts here! -- so, here it is.

More things need to be filled in yet -- for the other scalars I haven't
bothered to stamp out in this package yet, namely -- but these will be
cookie cutter and shan't be interesting.