go.tools/pointer: fix panic in reflection.

reflect.Values may point to tagged objects with
interface type, e.g. x := reflect.ValueOf(new(interface{})).Elem().
We failed to consider this when implementing Elem.

Also, (reflect.Value).Interface() must do one "unboxing"
when it encounters such tagged objects.
i.e., x.Elem().Interface() and x.Interface() are equivalent
in that case.

Also:
- add example of tagged object with interface type.
- untabify (Label).String docstring.
- added tests.

R=crawshaw
CC=golang-dev
https://golang.org/cl/18020044

pointer/doc.go

@@ -307,6 +307,15 @@ reflect.Value
   2) The dynamic type tag of a tagged object pointed to by a
      reflect.Value may be an interface type; it need not be concrete.
 
+     This arises in code such as this:
+        tEface := reflect.TypeOf(new(interface{}).Elem() // interface{}
+        eface := reflect.Zero(tEface)
+     pts(eface) is a singleton containing an interface{}-tagged
+     object.  That tagged object's payload is an interface{} value,
+     i.e. the pts of the payload contains only concrete-tagged
+     objects, although in this example it's the zero interface{} value,
+     so its pts is empty.
+
 reflect.Type
   Just as in the real "reflect" library, we represent a reflect.Type
   as an interface whose sole implementation is the concrete type,

pointer/labels.go

@@ -86,20 +86,20 @@ func (l Label) Pos() token.Pos {
 
 // String returns the printed form of this label.
 //
-// Examples:					Object type:
-// 	(sync.Mutex).Lock			(a function)
-// 	"foo":[]byte				(a slice constant)
-//	makemap					(map allocated via make)
-//	makechan				(channel allocated via make)
-//	makeinterface				(tagged object allocated by makeinterface)
-//      <alloc in reflect.Zero>			(allocation in instrinsic)
-//      sync.Mutex				(a reflect.rtype instance)
-//	<command-line arguments>		(an instrinsic object)
+// Examples:                                    Object type:
+//      (sync.Mutex).Lock                       (a function)
+//      "foo":[]byte                            (a slice constant)
+//      makemap                                 (map allocated via make)
+//      makechan                                (channel allocated via make)
+//      makeinterface                           (tagged object allocated by makeinterface)
+//      <alloc in reflect.Zero>                 (allocation in instrinsic)
+//      sync.Mutex                              (a reflect.rtype instance)
+//      <command-line arguments>                (an instrinsic object)
 //
 // Labels within compound objects have subelement paths:
-//	x.y[*].z				(a struct variable, x)
-//	append.y[*].z				(array allocated by append)
-//	makeslice.y[*].z			(array allocated via make)
+//      x.y[*].z                                (a struct variable, x)
+//      append.y[*].z                           (array allocated by append)
+//      makeslice.y[*].z                        (array allocated via make)
 //
 func (l Label) String() string {
 	var s string

pointer/reflect.go

@@ -12,6 +12,16 @@ package pointer
 // tagged objects.
 //
 // TODO(adonovan): all {} functions are TODO.
+//
+// TODO(adonovan): this file is rather subtle.  Explain how we derive
+// the implementation of each reflect operator from its spec,
+// including the subtleties of reflect.flag{Addr,RO,Indir}.
+// [Hint: our implementation is as if reflect.flagIndir was always
+// true, i.e. reflect.Values are pointers to tagged objects, there is
+// no inline allocation optimization; and indirect tagged objects (not
+// yet implemented) correspond to reflect.Values with
+// reflect.flagAddr.]
+// A picture would help too.
 
 import (
 	"fmt"
@@ -105,10 +115,9 @@ func (c *rVElemConstraint) solve(a *analysis, _ *node, delta nodeset) {
 
 		switch t := tDyn.Underlying().(type) {
 		case *types.Interface:
-			// A direct tagged object can't hold an
-			// interface type.  Implement when we support
-			// indirect tagged objects.
-			panic("unreachable")
+			if a.onlineCopy(c.result, payload) {
+				changed = true
+			}
 
 		case *types.Pointer:
 			obj := a.makeTagged(t.Elem(), c.cgn, nil)
@@ -215,18 +224,25 @@ func (c *rVInterfaceConstraint) solve(a *analysis, _ *node, delta nodeset) {
 	resultPts := &a.nodes[c.result].pts
 	changed := false
 	for vObj := range delta {
-		tDyn, _, indirect := a.taggedValue(vObj)
+		tDyn, payload, indirect := a.taggedValue(vObj)
 		if tDyn == nil {
 			panic("not a tagged object")
 		}
+
 		if indirect {
 			// TODO(adonovan): we'll need to implement this
 			// when we start creating indirect tagged objects.
 			panic("indirect tagged object")
 		}
 
-		if resultPts.add(vObj) {
-			changed = true
+		if _, ok := tDyn.Underlying().(*types.Interface); ok {
+			if a.onlineCopy(c.result, payload) {
+				a.addWork(c.result)
+			}
+		} else {
+			if resultPts.add(vObj) {
+				changed = true
+			}
 		}
 	}
 	if changed {

pointer/solve.go

@@ -128,7 +128,8 @@ func (a *analysis) solveConstraints(n *node, delta nodeset) {
 		if a.log != nil {
 			fmt.Fprintf(a.log, "\t\tconstraint %s\n", c)
 		}
-		// TODO(adonovan): parameter n is never used.  Remove?
+		// TODO(adonovan): parameter n is never needed, since
+		// it's equal to c.ptr().  Remove.
 		c.solve(a, n, delta)
 	}
 

pointer/testdata/arrayreflect.go

@@ -93,10 +93,15 @@ func reflectValueIndex() {
 }
 
 func reflectValueElem() {
-	// TODO(adonovan): tests 'interface'.  Needs indirect tagged objects.
-	// rv1 := reflect.ValueOf(...)
-	// print(rv1.Elem().Interface())        // #@types *int
-	// print(rv1.Elem().Interface().(*int)) // #@pointsto main.a
+	// Interface.
+	var iface interface{} = &a
+	rv1 := reflect.ValueOf(&iface).Elem()
+	print(rv1.Interface())               // @types *int
+	print(rv1.Interface().(*int))        // @pointsto main.a
+	print(rv1.Elem().Interface())        // @types *int
+	print(rv1.Elem().Interface().(*int)) // @pointsto main.a
+
+	print(reflect.ValueOf(new(interface{})).Elem().Elem()) // @types
 
 	// Pointer.
 	ptr := &a

pointer/testdata/reflect.go

@@ -44,7 +44,8 @@ func reflectTypeElem() {
 	print(reflect.Zero(reflect.TypeOf(make(map[string]float64)).Elem()).Interface()) // @types float64
 	print(reflect.Zero(reflect.TypeOf([3]complex64{}).Elem()).Interface())           // @types complex64
 	print(reflect.Zero(reflect.TypeOf(3).Elem()).Interface())                        // @types
-	print(reflect.Zero(reflect.TypeOf(new(interface{})).Elem()).Interface())         // @types interface{}
+	print(reflect.Zero(reflect.TypeOf(new(interface{})).Elem()))                     // @types interface{}
+	print(reflect.Zero(reflect.TypeOf(new(interface{})).Elem()).Interface())         // @types
 }
 
 func main() {

pointer/testdata/structreflect.go

@@ -21,12 +21,8 @@ func reflectTypeFieldByName() {
 	print(reflect.Zero(g.Type)) // @pointsto <alloc in reflect.Zero>
 	print(reflect.Zero(g.Type)) // @types interface{}
 
-	// TODO(adonovan): fix: the following should return a zero
-	// value of the empty interface (i.e. pts is empty), but that
-	// requires fixing the TODO comment in
-	// reflectZeroConstraint.solve, which in turn requires that we
-	// add a "settable" flag to tagged objects.
-	print(reflect.Zero(g.Type).Interface()) // @types interface{}
+	print(reflect.Zero(g.Type).Interface()) // @pointsto
+	print(reflect.Zero(g.Type).Interface()) // @types
 
 	h, _ := reflect.TypeOf(A{}).FieldByName("h")
 	print(h.Type) // @pointsto bool