Remarks on effects and visibility and wip states.

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.

_rsrch/nodesolution/impls/HACKME.md

@@ -108,3 +108,33 @@ Related to the above heading.
 
 Maps and lists in this package do their own internal handling of scalars,
 using unexported features inside the package, because they can more efficient.
+
+### when to invalidate the 'w' pointers
+
+The 'w' pointer -- short for 'wip' node pointer -- has an interesting lifecycle.
+
+In a NodeAssembler, the 'w' pointer should be intialized before the assembler is used.
+This means either the matching NodeBuilder type does so; or,
+if we're inside recursive structure, the parent assembler did so.
+
+The 'w' pointer is used throughout the life of the assembler.
+
+When assembly becomes "finished", the 'w' pointer should be set to nil,
+in order to make it impossible to continue to mutate that node.
+However, this doesn't *quite* work... because in the case of builders (at the root),
+we need to continue to hold onto the node between when it becomes "finished"
+and when Build is called, allowing us to return it (and then finally nil 'w').
+
+This has some kinda wild implications.  In recursive structures, it means the
+child assembler wrapper type is the one who takes reponsibility for nilling out
+the 'w' pointer at the same time as it updates the parent's state machine to
+proceed with the next entry.  In the case of scalars at the root of a build,
+it means *you can actually use the assign method more than once*.
+
+We could normalize the case with scalars at the root of a tree by adding another
+piece of memory to the scalar builders... but currently we haven't bothered.
+We're not in trouble on compositional correctness because nothing's visible
+until Build is called (whereas recursives have to be a lot stricter around this,
+because if something gets validated and finished, it's already essential that
+it now be unable to mutate out of the validated state, even if it's also not
+yet 'visible').

_rsrch/nodesolution/impls/map_K_T.go

@@ -386,7 +386,8 @@ func (ma *_Map_K_T__Assembler) AssembleDirectly(k string) (ipld.NodeAssembler, e
 	l := len(ma.w.t)
 	ma.w.t = append(ma.w.t, _Map_K_T__entry{k: K{k}})
 	ma.w.m[K{k}] = &ma.w.t[l].v
-	// Init the value assembler with a pointer to its target and yield it.
+	// Init the value assembler with a pointer to its target and to whole 'ma' and yield it.
+	ma.va.ma = ma
 	ma.va.ca.w = &ma.w.t[l].v
 	return &ma.va, nil
 }
@@ -400,7 +401,8 @@ func (ma *_Map_K_T__Assembler) AssembleKey() ipld.NodeAssembler {
 	// Extend entry table.
 	l := len(ma.w.t)
 	ma.w.t = append(ma.w.t, _Map_K_T__entry{})
-	// Init the key assembler with a pointer to its target and yield it.
+	// Init the key assembler with a pointer to its target and to whole 'ma' and yield it.
+	ma.ka.ma = ma
 	ma.ka.ca.w = &ma.w.t[l].k
 	return &ma.ka
 }
@@ -410,7 +412,8 @@ func (ma *_Map_K_T__Assembler) AssembleValue() ipld.NodeAssembler {
 		panic("misuse")
 	}
 	ma.state = maState_midValue
-	// Init the value assembler with a pointer to its target and yield it.
+	// Init the value assembler with a pointer to its targetand to whole 'ma' and yield it.
+	ma.va.ma = ma
 	ma.va.ca.w = &ma.w.t[len(ma.w.t)-1].v
 	return &ma.va
 }
@@ -450,6 +453,7 @@ func (mka *_Map_K_T__KeyAssembler) AssignString(v string) error {
 	mka.ma.w.m[K{v}] = &mka.ma.w.t[len(mka.ma.w.t)-1].v
 	// Update parent assembler state: clear to proceed.
 	mka.ma.state = maState_expectValue
+	mka.ma = nil // invalidate self to prevent further incorrect use.
 	return nil
 }
 func (_Map_K_T__KeyAssembler) AssignBytes([]byte) error   { panic("no") }
@@ -491,12 +495,14 @@ func (mva *_Map_K_T__ValueAssembler) AssignNode(v ipld.Node) error {
 }
 func (mva *_Map_K_T__ValueAssembler) flush() {
 	// The child assembler already assigned directly into the target memory,
-	//  and should have invalided its 'w' pointer when it was finished doing so,
 	//  so there's not much to do here... except update the assembler state machine.
-	// We also don't check the previous state because:
-	//  A) the appropriate time to do that would've been *before* assignments; and
-	//  B) if we were in a wrong state, we should've gotten a nil ptr panic when assigning 'w' content.
+	// We also don't check the previous state because context makes us already sure:
+	//  A) the appropriate time to do that would've been *before* assignments;
+	//  A.2) accordingly, we did so before exposing this value assembler at all; and
+	//  B) if we were in a wrong state because someone holds onto this too long,
+	//   the invalidation we're about to do on `mva.ma` will make .
 	mva.ma.state = maState_initial
+	mva.ma = nil // invalidate self to prevent further incorrect use.
 }
 func (_Map_K_T__ValueAssembler) Style() ipld.NodeStyle { panic("later") }
 

_rsrch/nodesolution/impls/map_test.go

@@ -151,6 +151,30 @@ func CheckMapStrInt(t *testing.T, ns ipld.NodeStyle) {
 			//  (and if it's clever, it'll differ from untyped, which will mean no assertion possible!).
 		}
 	})
+	t.Run("using expired child assemblers should panic", func(t *testing.T) {
+		nb := ns.NewBuilder()
+		ma, err := nb.BeginMap(3)
+		must.NotError(err)
+
+		// Assemble a key, and then try to assign it again.  Latter should fail.
+		ka := ma.AssembleKey()
+		must.NotError(ka.AssignString("whee"))
+		func() {
+			defer func() { recover() }()
+			ka.AssignString("woo")
+			t.Fatal("must not be reached")
+		}()
+
+		// Assemble a value, and then try to assign it again.  Latter should fail.
+		// (This does assume your system can continue after disregarding the last error.)
+		va := ma.AssembleValue()
+		must.NotError(va.AssignInt(1))
+		func() {
+			defer func() { recover() }()
+			va.AssignInt(2)
+			t.Fatal("must not be reached")
+		}()
+	})
 	t.Run("builder reset works", func(t *testing.T) {
 		// TODO
 	})