Node for all. Last phase deref.

Finally got the bind and free impls on the same page.

Surprisingly, the bind node can work without being a ptr itself.
I'm not sure if that'll last, but let's try to roll with it.
If we *can* keep that property, it might reduce GC impact in a pretty
significant way.

Added a 'fluent' package.  It's just a twinkle in my eye so far, but it
might represent the nicest way through all the discussed issues.
Nodes shouldn't have to be panicful; and users shouldn't have to do all
error handling manually either.  A package full of fluent interfaces
that know what's going on seems to be the only way to get both.
But we'll see how this shakes out.  Maybe typeful traversers will
make the whole thing end up more coreward than being relegated to a
helper package.  I have no idea.
Signed-off-by: Eric Myhre <hash@exultant.us>

fluent/fluentNode.go

+package fluent

impl/bind/boundNode.go

@@ -10,7 +10,7 @@ import (
 )
 
 var (
-	_ ipld.Node = &Node{}
+	_ ipld.Node = Node{}
 )
 
 /*
@@ -22,7 +22,7 @@ var (
 	If you're not sure which kind serializable node to use, try `ipldcbor.Node`.
 */
 type Node struct {
-	bound reflect.Value
+	bound reflect.Value // should already be ptr-unwrapped
 	atlas atlas.Atlas
 }
 
@@ -31,54 +31,40 @@ type Node struct {
 	atlas to understand how to traverse it.
 */
 func Bind(bindme interface{}, atl atlas.Atlas) ipld.Node {
-	return &Node{
-		bound: reflect.ValueOf(bindme),
+	rv := reflect.ValueOf(bindme)
+	for rv.Kind() == reflect.Ptr {
+		rv = rv.Elem()
+	}
+	return Node{
+		bound: rv,
 		atlas: atl,
 	}
 }
 
-func (n *Node) GetField(pth string) (v interface{}, _ error) {
-	return v, traverseField(n.bound, pth, n.atlas, reflect.ValueOf(v))
-}
-func (n *Node) GetFieldBool(pth string) (v bool, _ error) {
-	return v, traverseField(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
-}
-func (n *Node) GetFieldString(pth string) (v string, _ error) {
-	return v, traverseField(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
-}
-func (n *Node) GetFieldInt(pth string) (v int, _ error) {
-	return v, traverseField(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
-}
-func (n *Node) GetFieldLink(pth string) (v cid.Cid, _ error) {
-	return v, traverseField(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
-}
-
-func (n *Node) GetIndex(pth int) (v interface{}, _ error) {
-	return v, traverseIndex(n.bound, pth, n.atlas, reflect.ValueOf(v))
-}
-func (n *Node) GetIndexBool(pth int) (v bool, _ error) {
-	return v, traverseIndex(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
+func (n Node) AsBool() (v bool, _ error) {
+	reflect.ValueOf(&v).Elem().Set(n.bound)
+	return
 }
-func (n *Node) GetIndexString(pth int) (v string, _ error) {
-	return v, traverseIndex(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
+func (n Node) AsString() (v string, _ error) {
+	reflect.ValueOf(&v).Elem().Set(n.bound)
+	return
 }
-func (n *Node) GetIndexInt(pth int) (v int, _ error) {
-	return v, traverseIndex(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
+func (n Node) AsInt() (v int, _ error) {
+	reflect.ValueOf(&v).Elem().Set(n.bound)
+	return
 }
-func (n *Node) GetIndexLink(pth int) (v cid.Cid, _ error) {
-	return v, traverseIndex(n.bound, pth, n.atlas, reflect.ValueOf(&v).Elem())
+func (n Node) AsLink() (v cid.Cid, _ error) {
+	reflect.ValueOf(&v).Elem().Set(n.bound)
+	return
 }
 
-func traverseField(v reflect.Value, pth string, atl atlas.Atlas, assignTo reflect.Value) error {
-	// Unwrap any pointers.
-	for v.Kind() == reflect.Ptr {
-		v = v.Elem()
-	}
+func (n Node) TraverseField(pth string) (ipld.Node, error) {
+	v := n.bound
 
 	// Traverse.
 	//  Honor any atlent overrides if present;
 	//  Use kind-based fallbacks if necessary.
-	atlent, exists := atl.Get(reflect.ValueOf(v.Type()).Pointer())
+	atlent, exists := n.atlas.Get(reflect.ValueOf(v.Type()).Pointer())
 	if exists {
 		switch {
 		case atlent.MarshalTransformFunc != nil:
@@ -90,7 +76,7 @@ func traverseField(v reflect.Value, pth string, atl atlas.Atlas, assignTo reflec
 					break
 				}
 			}
-			return fmt.Errorf("traverse failed: type %q has no field named %q", v.Type().Name(), pth)
+			return Node{}, fmt.Errorf("traverse failed: type %q has no field named %q", v.Type().Name(), pth)
 		case atlent.UnionKeyedMorphism != nil:
 			panic(fmt.Errorf("invalid ipldbind.Node: type %q atlas specifies union, but ipld doesn't know how to make sense of this", v.Type().Name()))
 		case atlent.MapMorphism != nil:
@@ -114,7 +100,7 @@ func traverseField(v reflect.Value, pth string, atl atlas.Atlas, assignTo reflec
 			reflect.Map,
 			reflect.Struct,
 			reflect.Interface:
-			assignTo.Set(reflect.ValueOf(Node{v, atl}))
+			return Node{v, n.atlas}, nil
 		case // esotera: reject with panic
 			reflect.Chan,
 			reflect.Func,
@@ -136,23 +122,21 @@ func traverseField(v reflect.Value, pth string, atl atlas.Atlas, assignTo reflec
 	//  Or wrap with a Node and assign that (for recursives).
 	// TODO decide what to do with typedef'd primitives.
 	switch v.Type().Kind() {
-	case // primitives: set them
+	case // primitives: wrap in node
 		reflect.Bool,
 		reflect.String,
 		reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
 		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr,
 		reflect.Float32, reflect.Float64,
 		reflect.Complex64, reflect.Complex128:
-		assignTo.Set(v)
-		return nil
+		return Node{v, n.atlas}, nil
 	case // recursives: wrap in node
 		reflect.Array,
 		reflect.Slice,
 		reflect.Map,
 		reflect.Struct,
 		reflect.Interface:
-		assignTo.Set(reflect.ValueOf(Node{v, atl}))
-		return nil
+		return Node{v, n.atlas}, nil
 	case // esotera: reject with panic
 		reflect.Chan,
 		reflect.Func,
@@ -166,6 +150,6 @@ func traverseField(v reflect.Value, pth string, atl atlas.Atlas, assignTo reflec
 	}
 }
 
-func traverseIndex(v reflect.Value, pth int, atl atlas.Atlas, assignTo reflect.Value) error {
+func (n Node) TraverseIndex(idx int) (ipld.Node, error) {
 	panic("NYI")
 }

impl/free/freeNode.go

 package ipldfree
 
 import (
+	"fmt"
+	"strconv"
+
+	"github.com/ipfs/go-cid"
 	"github.com/ipld/go-ipld-prime"
 )
 
@@ -25,22 +29,24 @@ var (
 type Node struct {
 	typ typ
 
-	_map   map[string]interface{} // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_map2  map[int]interface{}    // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_arr   []interface{}          // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_str   string                 // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_bytes []byte                 // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_bool  bool                   // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_int   int64                  // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_uint  uint64                 // Value union.  Only one of these has meaning, depending on the value of 'Type'.
-	_float float64                // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	// n.b. maps values are all ptr so the returned node interface can be mutable.
+
+	_map   map[string]*Node // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_map2  map[int]*Node    // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_arr   []Node           // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_str   string           // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_bytes []byte           // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_bool  bool             // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_int   int              // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_uint  uint             // Value union.  Only one of these has meaning, depending on the value of 'Type'.
+	_float float64          // Value union.  Only one of these has meaning, depending on the value of 'Type'.
 
 }
 
 type typ struct{ t byte }
 
 var (
-	tZero  = typ{} // treat as tMap
+	tNil   = typ{}
 	tMap   = typ{'{'}
 	tArr   = typ{'['}
 	tStr   = typ{'s'}
@@ -49,8 +55,65 @@ var (
 	tInt   = typ{'i'}
 	tUint  = typ{'u'}
 	tFloat = typ{'f'}
+	tLink  = typ{'/'}
 )
 
-func (n *Node) GetField(pth []string) (v interface{}, _ error) {
-	return v, traverse(n.bound, pth, n.atlas, reflect.ValueOf(v))
+func (n *Node) AsBool() (v bool, _ error) {
+	return n._bool, expectTyp(tBool, n.typ)
+}
+func (n *Node) AsString() (v string, _ error) {
+	return n._str, expectTyp(tStr, n.typ)
+}
+func (n *Node) AsInt() (v int, _ error) {
+	return n._int, expectTyp(tInt, n.typ)
+}
+func (n *Node) AsLink() (v cid.Cid, _ error) {
+	return cid.Cid{}, expectTyp(tLink, n.typ) // TODO ??
+}
+
+func (n *Node) TraverseField(pth string) (ipld.Node, error) {
+	switch n.typ {
+	case tNil:
+		return nil, fmt.Errorf("cannot traverse terminals")
+	case tMap:
+		switch {
+		case n._map != nil:
+			v, _ := n._map[pth]
+			return v, nil
+		case n._map2 != nil:
+			i, err := strconv.Atoi(pth)
+			if err != nil {
+				return nil, fmt.Errorf("404")
+			}
+			v, _ := n._map2[i]
+			return v, nil
+		default:
+			panic("unreachable")
+		}
+	case tArr:
+		i, err := strconv.Atoi(pth)
+		if err != nil {
+			return nil, fmt.Errorf("404")
+		}
+		if i >= len(n._arr) {
+			return nil, fmt.Errorf("404")
+		}
+		v := &n._arr[i]
+		return v, nil
+	case tStr, tBytes, tBool, tInt, tUint, tFloat, tLink:
+		return nil, fmt.Errorf("cannot traverse terminals")
+	default:
+		panic("unreachable")
+	}
+}
+
+func (n *Node) TraverseIndex(idx int) (ipld.Node, error) {
+	panic("NYI")
+}
+
+func expectTyp(expect, actual typ) error {
+	if expect == actual {
+		return nil
+	}
+	return fmt.Errorf("woof")
 }

node.go

@@ -12,36 +12,18 @@ type Node interface {
 	// originally stored as a full node, you cannot immediately take
 	// a link to it for embedding in other objects (you'd have to make
 	// a new RootNode with the same content first, then store that).
-	//
-	// A variety of GetField{Foo} methods exists to yield specific types,
-	// and will panic upon encountering unexpected types.
-	GetField(path string) (interface{}, error)
-	GetFieldBool(path string) (bool, error)     // See GetField docs.
-	GetFieldString(path string) (string, error) // See GetField docs.
-	GetFieldInt(path string) (int, error)       // See GetField docs.
-	GetFieldLink(path string) (cid.Cid, error)  // See GetField docs.
+	TraverseField(path string) (Node, error)
 
 	// GetIndex is the equivalent of GetField but for indexing into an array
 	// (or a numerically-keyed map).  Like GetField, it returns
 	// either a primitive (e.g. string, int, etc), a link (type CID),
 	// or another Node.
-	//
-	// A variety of GetIndex{Foo} methods exists to yield specific types,
-	// and will panic upon encountering unexpected types.
-	GetIndex(idx int) (interface{}, error)
-	GetIndexBool(idx int) (bool, error)     // See GetIndex docs.
-	GetIndexString(idx int) (string, error) // See GetIndex docs.
-	GetIndexInt(idx int) (int, error)       // See GetIndex docs.
-	GetIndexLink(idx int) (cid.Cid, error)  // See GetIndex docs.
+	TraverseIndex(idx int) (Node, error)
 
-	// REVIEW this whole interface is still *very* unfriendly to chaining.
-	// Friendly would be returning `(Node)` at all times, and having final
-	// dereferencing options on leaf nodes, and treating it as a Maybe at all
-	// other times.  Multireturn methods are antithetical to graceful fluent
-	// chaining in golang, syntactically.
-	// Panics might also be a viable option.  I suspect we do quite usually
-	// want to do large amounts of these traversal operations, and bailing at
-	// any point is desired to be terse.  We could provide a thunkwrapper.
+	AsBool() (bool, error)
+	AsString() (string, error)
+	AsInt() (int, error)
+	AsLink() (cid.Cid, error)
 }
 
 type SerializableNode interface {