compiler: implement method-set based AssignableTo and Implements (#5304)

* reflect: implement method-set based AssignableTo and Implements

Based on the design from #4376 by aykevl.
Fixes #4277, fixes #3580.

Co-authored-by: Ayke van Laethem <aykevanlaethem@gmail.com>

* builder: update expected binary sizes for reflect changes

* Make interface checks similar to invoke, allowing typeImplementsMethodSet and method info to be dropped when reflect is not present

* Add more tests that BigGo reflect tests

* Even more pruning

* Add go/token and net/url to passing tests

* Prune even further, I am less happy with this, though

* Update size test now that we are smaller

* Skip some tests

* elide method lists

* format, oops

* fix tests

* Add a panic, pull out constant to keep in sync

* Add debug info

* Remove code that was leftover from a previous refactor

---------

Co-authored-by: Ayke van Laethem <aykevanlaethem@gmail.com>

Author: jakebailey

GNUmakefile

@@ -348,6 +348,7 @@ TEST_PACKAGES_FAST = \
 	go/ast \
 	go/format \
 	go/scanner \
+	go/token \
 	go/version \
 	hash \
 	hash/adler32 \
@@ -360,6 +361,7 @@ TEST_PACKAGES_FAST = \
 	math/cmplx \
 	net/http/internal/ascii \
 	net/mail \
+	net/url \
 	os \
 	path \
 	reflect \
@@ -485,7 +487,7 @@ TEST_PACKAGES_HOST := $(TEST_PACKAGES_FAST) $(TEST_PACKAGES_WINDOWS)
 TEST_IOFS := false
 endif
 
-TEST_SKIP_FLAG := -skip='TestExtraMethods|TestParseAndBytesRoundTrip/P256/Generic'
+TEST_SKIP_FLAG := -skip='TestExtraMethods|TestParseAndBytesRoundTrip/P256/Generic|TestParseQueryLimits|TestParseStrictIpv6'
 TEST_ADDITIONAL_FLAGS ?=
 
 # Test known-working standard library packages.

compiler/interface.go

@@ -10,13 +10,20 @@ import (
 	"fmt"
 	"go/token"
 	"go/types"
+	"sort"
 	"strconv"
 	"strings"
 
 	"golang.org/x/tools/go/ssa"
 	"tinygo.org/x/go-llvm"
 )
 
+// numMethodHasMethodSet is a flag in bit 15 of the numMethod field (uint16) in
+// Named, Pointer, and Struct type descriptors. When set, an inline method set
+// is present in the type descriptor. Must match the constant in
+// src/internal/reflectlite/type.go.
+const numMethodHasMethodSet = 0x8000
+
 // Type kinds for basic types.
 // They must match the constants for the Kind type in src/reflect/type.go.
 var basicTypes = [...]uint8{
@@ -183,6 +190,16 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 		typeFieldTypes := []*types.Var{
 			types.NewVar(token.NoPos, nil, "kind", types.Typ[types.Int8]),
 		}
+		// Compute the method set value for types that support methods.
+		var methods []*types.Func
+		for i := 0; i < ms.Len(); i++ {
+			methods = append(methods, ms.At(i).Obj().(*types.Func))
+		}
+		methodSetType := types.NewStruct([]*types.Var{
+			types.NewVar(token.NoPos, nil, "length", types.Typ[types.Uintptr]),
+			types.NewVar(token.NoPos, nil, "methods", types.NewArray(types.Typ[types.UnsafePointer], int64(len(methods)))),
+		}, nil)
+		methodSetValue := c.getMethodSetValue(methods)
 		switch typ := typ.(type) {
 		case *types.Basic:
 			typeFieldTypes = append(typeFieldTypes,
@@ -199,6 +216,13 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				types.NewVar(token.NoPos, nil, "ptrTo", types.Typ[types.UnsafePointer]),
 				types.NewVar(token.NoPos, nil, "underlying", types.Typ[types.UnsafePointer]),
 				types.NewVar(token.NoPos, nil, "pkgpath", types.Typ[types.UnsafePointer]),
+			)
+			if len(methods) > 0 {
+				typeFieldTypes = append(typeFieldTypes,
+					types.NewVar(token.NoPos, nil, "methods", methodSetType),
+				)
+			}
+			typeFieldTypes = append(typeFieldTypes,
 				types.NewVar(token.NoPos, nil, "name", types.NewArray(types.Typ[types.Int8], int64(len(pkgname)+1+len(name)+1))),
 			)
 		case *types.Chan:
@@ -218,6 +242,11 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				types.NewVar(token.NoPos, nil, "numMethods", types.Typ[types.Uint16]),
 				types.NewVar(token.NoPos, nil, "elementType", types.Typ[types.UnsafePointer]),
 			)
+			if len(methods) > 0 {
+				typeFieldTypes = append(typeFieldTypes,
+					types.NewVar(token.NoPos, nil, "methods", methodSetType),
+				)
+			}
 		case *types.Array:
 			typeFieldTypes = append(typeFieldTypes,
 				types.NewVar(token.NoPos, nil, "numMethods", types.Typ[types.Uint16]),
@@ -242,11 +271,16 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				types.NewVar(token.NoPos, nil, "numFields", types.Typ[types.Uint16]),
 				types.NewVar(token.NoPos, nil, "fields", types.NewArray(c.getRuntimeType("structField"), int64(typ.NumFields()))),
 			)
+			if len(methods) > 0 {
+				typeFieldTypes = append(typeFieldTypes,
+					types.NewVar(token.NoPos, nil, "methods", methodSetType),
+				)
+			}
 		case *types.Interface:
 			typeFieldTypes = append(typeFieldTypes,
 				types.NewVar(token.NoPos, nil, "ptrTo", types.Typ[types.UnsafePointer]),
+				types.NewVar(token.NoPos, nil, "methods", methodSetType),
 			)
-			// TODO: methods
 		case *types.Signature:
 			typeFieldTypes = append(typeFieldTypes,
 				types.NewVar(token.NoPos, nil, "ptrTo", types.Typ[types.UnsafePointer]),
@@ -292,14 +326,24 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				pkgname = pkg.Name()
 			}
 			pkgPathPtr := c.pkgPathPtr(pkgpath)
+			namedNumMethods := uint64(numMethods)
+			if namedNumMethods&numMethodHasMethodSet != 0 {
+				panic("numMethods overflow: too many exported methods on named type " + name)
+			}
+			if len(methods) > 0 {
+				namedNumMethods |= numMethodHasMethodSet
+			}
 			typeFields = []llvm.Value{
-				llvm.ConstInt(c.ctx.Int16Type(), uint64(numMethods), false), // numMethods
-				c.getTypeCode(types.NewPointer(typ)),                        // ptrTo
-				c.getTypeCode(typ.Underlying()),                             // underlying
-				pkgPathPtr,                                                  // pkgpath pointer
-				c.ctx.ConstString(pkgname+"."+name+"\x00", false),           // name
+				llvm.ConstInt(c.ctx.Int16Type(), namedNumMethods, false), // numMethods
+				c.getTypeCode(types.NewPointer(typ)),                     // ptrTo
+				c.getTypeCode(typ.Underlying()),                          // underlying
+				pkgPathPtr,                                               // pkgpath pointer
 			}
-			metabyte |= 1 << 5 // "named" flag
+			if len(methods) > 0 {
+				typeFields = append(typeFields, methodSetValue) // methods
+			}
+			typeFields = append(typeFields, c.ctx.ConstString(pkgname+"."+name+"\x00", false)) // name
+			metabyte |= 1 << 5                                                                 // "named" flag
 		case *types.Chan:
 			var dir reflectChanDir
 			switch typ.Dir() {
@@ -323,10 +367,20 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				c.getTypeCode(typ.Elem()),                  // elementType
 			}
 		case *types.Pointer:
+			ptrNumMethods := uint64(numMethods)
+			if ptrNumMethods&numMethodHasMethodSet != 0 {
+				panic("numMethods overflow: too many exported methods on pointer type")
+			}
+			if len(methods) > 0 {
+				ptrNumMethods |= numMethodHasMethodSet
+			}
 			typeFields = []llvm.Value{
-				llvm.ConstInt(c.ctx.Int16Type(), uint64(numMethods), false), // numMethods
+				llvm.ConstInt(c.ctx.Int16Type(), ptrNumMethods, false), // numMethods
 				c.getTypeCode(typ.Elem()),
 			}
+			if len(methods) > 0 {
+				typeFields = append(typeFields, methodSetValue)
+			}
 		case *types.Array:
 			typeFields = []llvm.Value{
 				llvm.ConstInt(c.ctx.Int16Type(), 0, false),             // numMethods
@@ -353,9 +407,16 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 
 			llvmStructType := c.getLLVMType(typ)
 			size := c.targetData.TypeStoreSize(llvmStructType)
+			structNumMethods := uint64(numMethods)
+			if structNumMethods&numMethodHasMethodSet != 0 {
+				panic("numMethods overflow: too many exported methods on struct type")
+			}
+			if len(methods) > 0 {
+				structNumMethods |= numMethodHasMethodSet
+			}
 			typeFields = []llvm.Value{
-				llvm.ConstInt(c.ctx.Int16Type(), uint64(numMethods), false), // numMethods
-				c.getTypeCode(types.NewPointer(typ)),                        // ptrTo
+				llvm.ConstInt(c.ctx.Int16Type(), structNumMethods, false), // numMethods
+				c.getTypeCode(types.NewPointer(typ)),                      // ptrTo
 				pkgPathPtr,
 				llvm.ConstInt(c.ctx.Int32Type(), uint64(size), false),            // size
 				llvm.ConstInt(c.ctx.Int16Type(), uint64(typ.NumFields()), false), // numFields
@@ -407,9 +468,14 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
 				}))
 			}
 			typeFields = append(typeFields, llvm.ConstArray(structFieldType, fields))
+			if len(methods) > 0 {
+				typeFields = append(typeFields, methodSetValue)
+			}
 		case *types.Interface:
-			typeFields = []llvm.Value{c.getTypeCode(types.NewPointer(typ))}
-			// TODO: methods
+			typeFields = []llvm.Value{
+				c.getTypeCode(types.NewPointer(typ)),
+				methodSetValue,
+			}
 		case *types.Signature:
 			typeFields = []llvm.Value{c.getTypeCode(types.NewPointer(typ))}
 			// TODO: params, return values, etc
@@ -696,17 +762,11 @@ func (b *builder) createTypeAssert(expr *ssa.TypeAssert) llvm.Value {
 			// This type assertion always succeeds, so we can just set commaOk to true.
 			commaOk = llvm.ConstInt(b.ctx.Int1Type(), 1, true)
 		} else {
-			// Type assert on interface type with methods.
-			// This is a call to an interface type assert function.
-			// The interface lowering pass will define this function by filling it
-			// with a type switch over all concrete types that implement this
-			// interface, and returning whether it's one of the matched types.
-			// This is very different from how interface asserts are implemented in
-			// the main Go compiler, where the runtime checks whether the type
-			// implements each method of the interface. See:
-			// https://research.swtch.com/interfaces
-			fn := b.getInterfaceImplementsFunc(expr.AssertedType)
-			commaOk = b.CreateCall(fn.GlobalValueType(), fn, []llvm.Value{actualTypeNum}, "")
+			// Type assert on an interface type with methods.
+			// Create a call to a declared-but-not-defined function that will
+			// be lowered by the interface lowering pass into a type-ID
+			// comparison chain.
+			commaOk = b.createInterfaceTypeAssert(intf, actualTypeNum)
 		}
 	} else {
 		name, _ := getTypeCodeName(expr.AssertedType)
@@ -783,20 +843,58 @@ func (c *compilerContext) getMethodsString(itf *types.Interface) string {
 	return strings.Join(methods, "; ")
 }
 
-// getInterfaceImplementsFunc returns a declared function that works as a type
-// switch. The interface lowering pass will define this function.
-func (c *compilerContext) getInterfaceImplementsFunc(assertedType types.Type) llvm.Value {
-	s, _ := getTypeCodeName(assertedType.Underlying())
-	fnName := s + ".$typeassert"
-	llvmFn := c.mod.NamedFunction(fnName)
-	if llvmFn.IsNil() {
-		llvmFnType := llvm.FunctionType(c.ctx.Int1Type(), []llvm.Type{c.dataPtrType}, false)
-		llvmFn = llvm.AddFunction(c.mod, fnName, llvmFnType)
-		c.addStandardDeclaredAttributes(llvmFn)
-		methods := c.getMethodsString(assertedType.Underlying().(*types.Interface))
-		llvmFn.AddFunctionAttr(c.ctx.CreateStringAttribute("tinygo-methods", methods))
+// getMethodSetValue creates the method set struct value for a list of methods.
+// The struct contains a length and a sorted array of method signature pointers.
+func (c *compilerContext) getMethodSetValue(methods []*types.Func) llvm.Value {
+	// Create a sorted list of method signature global names.
+	type methodRef struct {
+		name  string
+		value llvm.Value
 	}
-	return llvmFn
+	var refs []methodRef
+	for _, method := range methods {
+		name := method.Name()
+		if !token.IsExported(name) {
+			name = method.Pkg().Path() + "." + name
+		}
+		s, _ := getTypeCodeName(method.Type())
+		globalName := "reflect/types.signature:" + name + ":" + s
+		value := c.mod.NamedGlobal(globalName)
+		if value.IsNil() {
+			value = llvm.AddGlobal(c.mod, c.ctx.Int8Type(), globalName)
+			value.SetInitializer(llvm.ConstNull(c.ctx.Int8Type()))
+			value.SetGlobalConstant(true)
+			value.SetLinkage(llvm.LinkOnceODRLinkage)
+			value.SetAlignment(1)
+			if c.Debug {
+				file := c.getDIFile("<Go type>")
+				diglobal := c.dibuilder.CreateGlobalVariableExpression(file, llvm.DIGlobalVariableExpression{
+					Name:        globalName,
+					File:        file,
+					Line:        1,
+					Type:        c.getDIType(types.Typ[types.Uint8]),
+					LocalToUnit: false,
+					Expr:        c.dibuilder.CreateExpression(nil),
+					AlignInBits: 8,
+				})
+				value.AddMetadata(0, diglobal)
+			}
+		}
+		refs = append(refs, methodRef{globalName, value})
+	}
+	sort.Slice(refs, func(i, j int) bool {
+		return refs[i].name < refs[j].name
+	})
+
+	var values []llvm.Value
+	for _, ref := range refs {
+		values = append(values, ref.value)
+	}
+
+	return c.ctx.ConstStruct([]llvm.Value{
+		llvm.ConstInt(c.uintptrType, uint64(len(values)), false),
+		llvm.ConstArray(c.dataPtrType, values),
+	}, false)
 }
 
 // getInvokeFunction returns the thunk to call the given interface method. The
@@ -823,6 +921,24 @@ func (c *compilerContext) getInvokeFunction(instr *ssa.CallCommon) llvm.Value {
 	return llvmFn
 }
 
+// createInterfaceTypeAssert creates a call to a declared-but-not-defined
+// $typeassert function for the given interface. This function will be defined
+// by the interface lowering pass as a type-ID comparison chain, avoiding the
+// need for runtime.typeImplementsMethodSet at compile time.
+func (b *builder) createInterfaceTypeAssert(intf *types.Interface, actualType llvm.Value) llvm.Value {
+	s, _ := getTypeCodeName(intf)
+	fnName := s + ".$typeassert"
+	llvmFn := b.mod.NamedFunction(fnName)
+	if llvmFn.IsNil() {
+		llvmFnType := llvm.FunctionType(b.ctx.Int1Type(), []llvm.Type{b.dataPtrType}, false)
+		llvmFn = llvm.AddFunction(b.mod, fnName, llvmFnType)
+		b.addStandardDeclaredAttributes(llvmFn)
+		methods := b.getMethodsString(intf)
+		llvmFn.AddFunctionAttr(b.ctx.CreateStringAttribute("tinygo-methods", methods))
+	}
+	return b.CreateCall(llvmFn.GlobalValueType(), llvmFn, []llvm.Value{actualType}, "")
+}
+
 // getInterfaceInvokeWrapper returns a wrapper for the given method so it can be
 // invoked from an interface. The wrapper takes in a pointer to the underlying
 // value, dereferences or unpacks it if necessary, and calls the real method.

compiler/testdata/interface.ll

@@ -9,12 +9,14 @@ target triple = "wasm32-unknown-wasi"
 @"reflect/types.type:basic:int" = linkonce_odr constant { i8, ptr } { i8 -62, ptr @"reflect/types.type:pointer:basic:int" }, align 4
 @"reflect/types.type:pointer:basic:int" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:basic:int" }, align 4
 @"reflect/types.type:pointer:named:error" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:named:error" }, align 4
-@"reflect/types.type:named:error" = linkonce_odr constant { i8, i16, ptr, ptr, ptr, [7 x i8] } { i8 116, i16 1, ptr @"reflect/types.type:pointer:named:error", ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}", ptr @"reflect/types.type.pkgpath.empty", [7 x i8] c".error\00" }, align 4
+@"reflect/types.signature:Error:func:{}{basic:string}" = linkonce_odr constant i8 0, align 1
+@"reflect/types.type:named:error" = linkonce_odr constant { i8, i16, ptr, ptr, ptr, { i32, [1 x ptr] }, [7 x i8] } { i8 116, i16 -32767, ptr @"reflect/types.type:pointer:named:error", ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}", ptr @"reflect/types.type.pkgpath.empty", { i32, [1 x ptr] } { i32 1, [1 x ptr] [ptr @"reflect/types.signature:Error:func:{}{basic:string}"] }, [7 x i8] c".error\00" }, align 4
 @"reflect/types.type.pkgpath.empty" = linkonce_odr unnamed_addr constant [1 x i8] zeroinitializer, align 1
-@"reflect/types.type:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr } { i8 84, ptr @"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}" }, align 4
+@"reflect/types.type:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr, { i32, [1 x ptr] } } { i8 84, ptr @"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}", { i32, [1 x ptr] } { i32 1, [1 x ptr] [ptr @"reflect/types.signature:Error:func:{}{basic:string}"] } }, align 4
 @"reflect/types.type:pointer:interface:{Error:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:interface:{Error:func:{}{basic:string}}" }, align 4
 @"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, i16, ptr } { i8 -43, i16 0, ptr @"reflect/types.type:interface:{String:func:{}{basic:string}}" }, align 4
-@"reflect/types.type:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr } { i8 84, ptr @"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}" }, align 4
+@"reflect/types.signature:String:func:{}{basic:string}" = linkonce_odr constant i8 0, align 1
+@"reflect/types.type:interface:{String:func:{}{basic:string}}" = linkonce_odr constant { i8, ptr, { i32, [1 x ptr] } } { i8 84, ptr @"reflect/types.type:pointer:interface:{String:func:{}{basic:string}}", { i32, [1 x ptr] } { i32 1, [1 x ptr] [ptr @"reflect/types.signature:String:func:{}{basic:string}"] } }, align 4
 @"reflect/types.typeid:basic:int" = external constant i8
 
 ; Function Attrs: allockind("alloc,zeroed") allocsize(0)

interp/interpreter.go

@@ -413,64 +413,6 @@ func (r *runner) run(fn *function, params []value, parentMem *memoryView, indent
 				// Special function that will trigger an error.
 				// This is used to test error reporting.
 				return nil, mem, r.errorAt(inst, errors.New("test error"))
-			case strings.HasSuffix(callFn.name, ".$typeassert"):
-				if r.debug {
-					fmt.Fprintln(os.Stderr, indent+"interface assert:", operands[1:])
-				}
-
-				// Load various values for the interface implements check below.
-				typecodePtr, err := operands[1].asPointer(r)
-				if err != nil {
-					return nil, mem, r.errorAt(inst, err)
-				}
-				// typecodePtr always point to the numMethod field in the type
-				// description struct. The methodSet, when present, comes right
-				// before the numMethod field (the compiler doesn't generate
-				// method sets for concrete types without methods).
-				// Considering that the compiler doesn't emit interface type
-				// asserts for interfaces with no methods (as the always succeed)
-				// then if the offset is zero, this assert must always fail.
-				if typecodePtr.offset() == 0 {
-					locals[inst.localIndex] = literalValue{uint8(0)}
-					break
-				}
-				typecodePtrOffset, err := typecodePtr.addOffset(-int64(r.pointerSize))
-				if err != nil {
-					return nil, mem, r.errorAt(inst, err)
-				}
-				methodSetPtr, err := mem.load(typecodePtrOffset, r.pointerSize).asPointer(r)
-				if err != nil {
-					return nil, mem, r.errorAt(inst, err)
-				}
-				methodSet := mem.get(methodSetPtr.index()).llvmGlobal.Initializer()
-				numMethods := int(r.builder.CreateExtractValue(methodSet, 0, "").ZExtValue())
-				llvmFn := inst.llvmInst.CalledValue()
-				methodSetAttr := llvmFn.GetStringAttributeAtIndex(-1, "tinygo-methods")
-				methodSetString := methodSetAttr.GetStringValue()
-
-				// Make a set of all the methods on the concrete type, for
-				// easier checking in the next step.
-				concreteTypeMethods := map[string]struct{}{}
-				for i := 0; i < numMethods; i++ {
-					methodInfo := r.builder.CreateExtractValue(methodSet, 1, "")
-					name := r.builder.CreateExtractValue(methodInfo, i, "").Name()
-					concreteTypeMethods[name] = struct{}{}
-				}
-
-				// Check whether all interface methods are also in the list
-				// of defined methods calculated above. This is the interface
-				// assert itself.
-				assertOk := uint8(1) // i1 true
-				for _, name := range strings.Split(methodSetString, "; ") {
-					if _, ok := concreteTypeMethods[name]; !ok {
-						// There is a method on the interface that is not
-						// implemented by the type. The assertion will fail.
-						assertOk = 0 // i1 false
-						break
-					}
-				}
-				// If assertOk is still 1, the assertion succeeded.
-				locals[inst.localIndex] = literalValue{assertOk}
 			case strings.HasSuffix(callFn.name, "$invoke"):
 				// This thunk is the interface method dispatcher: it is called
 				// with all regular parameters and a type code. It will then