[Types] Handle function types fully in more places (#3381)

Call isFunction to check for a general function type instead of just
a funcref, in places where we care about both, and some other minor
miscellaneous typing fixes in preparation for typed function references
(this will be tested fully at that time).

Change is mostly whitespace.

5 files changed, 120 insertions, 101 deletions

diff --git a/src/passes/InstrumentLocals.cpp b/src/passes/InstrumentLocals.cpp
index 4b0db87ed..81494463f 100644
--- a/src/passes/InstrumentLocals.cpp
+++ b/src/passes/InstrumentLocals.cpp
@@ -134,44 +134,46 @@ struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
 
     Builder builder(*getModule());
     Name import;
-    TODO_SINGLE_COMPOUND(curr->value->type);
-    switch (curr->value->type.getBasic()) {
-      case Type::i32:
-        import = set_i32;
-        break;
-      case Type::i64:
-        return; // TODO
-      case Type::f32:
-        import = set_f32;
-        break;
-      case Type::f64:
-        import = set_f64;
-        break;
-      case Type::v128:
-        import = set_v128;
-        break;
-      case Type::funcref:
-        import = set_funcref;
-        break;
-      case Type::externref:
-        import = set_externref;
-        break;
-      case Type::exnref:
-        import = set_exnref;
-        break;
-      case Type::anyref:
-        import = set_anyref;
-        break;
-      case Type::eqref:
-        import = set_eqref;
-        break;
-      case Type::i31ref:
-        import = set_i31ref;
-        break;
-      case Type::unreachable:
-        return; // nothing to do here
-      case Type::none:
-        WASM_UNREACHABLE("unexpected type");
+    auto type = curr->value->type;
+    if (type.isFunction()) {
+      import = set_funcref;
+    } else {
+      TODO_SINGLE_COMPOUND(curr->value->type);
+      switch (type.getBasic()) {
+        case Type::i32:
+          import = set_i32;
+          break;
+        case Type::i64:
+          return; // TODO
+        case Type::f32:
+          import = set_f32;
+          break;
+        case Type::f64:
+          import = set_f64;
+          break;
+        case Type::v128:
+          import = set_v128;
+          break;
+        case Type::externref:
+          import = set_externref;
+          break;
+        case Type::exnref:
+          import = set_exnref;
+          break;
+        case Type::anyref:
+          import = set_anyref;
+          break;
+        case Type::eqref:
+          import = set_eqref;
+          break;
+        case Type::i31ref:
+          import = set_i31ref;
+          break;
+        case Type::unreachable:
+          return; // nothing to do here
+        default:
+          WASM_UNREACHABLE("unexpected type");
+      }
     }
     curr->value = builder.makeCall(import,
                                    {builder.makeConst(int32_t(id++)),
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index 75e73a834..d448cbd94 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -741,24 +741,29 @@ public:
   // Make a constant expression. This might be a wasm Const, or something
   // else of constant value like ref.null.
   Expression* makeConstantExpression(Literal value) {
-    TODO_SINGLE_COMPOUND(value.type);
-    switch (value.type.getBasic()) {
-      case Type::funcref:
-        if (!value.isNull()) {
-          return makeRefFunc(value.getFunc());
-        }
-        return makeRefNull(value.type);
+    auto type = value.type;
+    if (type.isNumber()) {
+      return makeConst(value);
+    }
+    if (type.isFunction()) {
+      if (!value.isNull()) {
+        // TODO: with typed function references we need to do more for the type
+        return makeRefFunc(value.getFunc());
+      }
+      return makeRefNull(type);
+    }
+    TODO_SINGLE_COMPOUND(type);
+    switch (type.getBasic()) {
       case Type::externref:
       case Type::exnref: // TODO: ExceptionPackage?
       case Type::anyref:
       case Type::eqref:
         assert(value.isNull() && "unexpected non-null reference type literal");
-        return makeRefNull(value.type);
+        return makeRefNull(type);
       case Type::i31ref:
         return makeI31New(makeConst(value.geti31()));
       default:
-        assert(value.type.isNumber());
-        return makeConst(value);
+        WASM_UNREACHABLE("invalid constant expression");
     }
   }
 
@@ -923,6 +928,9 @@ public:
     if (curr->type.isTuple()) {
       return makeConstantExpression(Literal::makeZeros(curr->type));
     }
+    if (curr->type.isFunction()) {
+      return ExpressionManipulator::refNull(curr, curr->type);
+    }
     Literal value;
     // TODO: reuse node conditionally when possible for literals
     TODO_SINGLE_COMPOUND(curr->type);
@@ -946,6 +954,7 @@ public:
         break;
       }
       case Type::funcref:
+        WASM_UNREACHABLE("handled above");
       case Type::externref:
       case Type::exnref:
       case Type::anyref:
diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp
index 6613bc1c7..c80bb44b8 100644
--- a/src/wasm/literal.cpp
+++ b/src/wasm/literal.cpp
@@ -422,58 +422,59 @@ void Literal::printVec128(std::ostream& o, const std::array<uint8_t, 16>& v) {
 
 std::ostream& operator<<(std::ostream& o, Literal literal) {
   prepareMinorColor(o);
-  TODO_SINGLE_COMPOUND(literal.type);
-  switch (literal.type.getBasic()) {
-    case Type::none:
-      o << "?";
-      break;
-    case Type::i32:
-      o << literal.geti32();
-      break;
-    case Type::i64:
-      o << literal.geti64();
-      break;
-    case Type::f32:
-      literal.printFloat(o, literal.getf32());
-      break;
-    case Type::f64:
-      literal.printDouble(o, literal.getf64());
-      break;
-    case Type::v128:
-      o << "i32x4 ";
-      literal.printVec128(o, literal.getv128());
-      break;
-    case Type::funcref:
-      if (literal.isNull()) {
-        o << "funcref(null)";
-      } else {
-        o << "funcref(" << literal.getFunc() << ")";
-      }
-      break;
-    case Type::externref:
-      assert(literal.isNull() && "unexpected non-null externref literal");
-      o << "externref(null)";
-      break;
-    case Type::exnref:
-      if (literal.isNull()) {
-        o << "exnref(null)";
-      } else {
-        o << "exnref(" << literal.getExceptionPackage() << ")";
-      }
-      break;
-    case Type::anyref:
-      assert(literal.isNull() && "unexpected non-null anyref literal");
-      o << "anyref(null)";
-      break;
-    case Type::eqref:
-      assert(literal.isNull() && "unexpected non-null eqref literal");
-      o << "eqref(null)";
-      break;
-    case Type::i31ref:
-      o << "i31ref(" << literal.geti31() << ")";
-      break;
-    case Type::unreachable:
-      WASM_UNREACHABLE("invalid type");
+  if (literal.type.isFunction()) {
+    if (literal.isNull()) {
+      o << "funcref(null)";
+    } else {
+      o << "funcref(" << literal.getFunc() << ")";
+    }
+  } else {
+    TODO_SINGLE_COMPOUND(literal.type);
+    switch (literal.type.getBasic()) {
+      case Type::none:
+        o << "?";
+        break;
+      case Type::i32:
+        o << literal.geti32();
+        break;
+      case Type::i64:
+        o << literal.geti64();
+        break;
+      case Type::f32:
+        literal.printFloat(o, literal.getf32());
+        break;
+      case Type::f64:
+        literal.printDouble(o, literal.getf64());
+        break;
+      case Type::v128:
+        o << "i32x4 ";
+        literal.printVec128(o, literal.getv128());
+        break;
+      case Type::externref:
+        assert(literal.isNull() && "unexpected non-null externref literal");
+        o << "externref(null)";
+        break;
+      case Type::exnref:
+        if (literal.isNull()) {
+          o << "exnref(null)";
+        } else {
+          o << "exnref(" << literal.getExceptionPackage() << ")";
+        }
+        break;
+      case Type::anyref:
+        assert(literal.isNull() && "unexpected non-null anyref literal");
+        o << "anyref(null)";
+        break;
+      case Type::eqref:
+        assert(literal.isNull() && "unexpected non-null eqref literal");
+        o << "eqref(null)";
+        break;
+      case Type::i31ref:
+        o << "i31ref(" << literal.geti31() << ")";
+        break;
+      default:
+        WASM_UNREACHABLE("invalid type");
+    }
   }
   restoreNormalColor(o);
   return o;
diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp
index 5e4d5e07f..dc4d50ef4 100644
--- a/src/wasm/wasm-type.cpp
+++ b/src/wasm/wasm-type.cpp
@@ -579,6 +579,9 @@ Type Type::getLeastUpperBound(Type a, Type b) {
   }
   if (a.isRef()) {
     if (b.isRef()) {
+      if (a.isFunction() && b.isFunction()) {
+        return Type::funcref;
+      }
       if ((a == Type::i31ref && b == Type::eqref) ||
           (a == Type::eqref && b == Type::i31ref)) {
         return Type::eqref;
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 38dfa8092..809ca5a6a 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -2030,6 +2030,10 @@ void FunctionValidator::visitRefFunc(RefFunc* curr) {
   }
   auto* func = getModule()->getFunctionOrNull(curr->func);
   shouldBeTrue(!!func, curr, "function argument of ref.func must exist");
+  shouldBeTrue(curr->type.isFunction(),
+               curr,
+               "ref.func must have a function reference type");
+  // TODO: check for non-nullability
 }
 
 void FunctionValidator::visitRefEq(RefEq* curr) {
@@ -2311,7 +2315,7 @@ void FunctionValidator::visitFunction(Function* curr) {
     shouldBeTrue(var.isConcrete(), curr, "vars must be concretely typed");
   }
   shouldBeTrue(features <= getModule()->features,
-               curr,
+               curr->name,
                "all used types should be allowed");
   if (curr->profile == IRProfile::Poppy) {
     shouldBeTrue(