Remove ref.is_null type parameter (#1474)

binji · web-flow · commit bd52b8845aa8 · 2020-07-15T16:39:52.000-07:00
See WebAssembly/reference-types#99. This change also updates the testsuite, so the spec tests pass too. In addition, the behavior of `br_table` is no longer different from MVP, and has a text to confirm this. That is now fixed in `type-checker.cc` too.
diff --git a/src/binary-reader-ir.cc b/src/binary-reader-ir.cc
@@ -185,7 +185,7 @@ class BinaryReaderIR : public BinaryReaderNop {
   Result OnTableFillExpr(Index table_index) override;
   Result OnRefFuncExpr(Index func_index) override;
   Result OnRefNullExpr(Type type) override;
-  Result OnRefIsNullExpr(Type type) override;
+  Result OnRefIsNullExpr() override;
   Result OnNopExpr() override;
   Result OnRethrowExpr() override;
   Result OnReturnExpr() override;
@@ -919,8 +919,8 @@ Result BinaryReaderIR::OnRefNullExpr(Type type) {
   return AppendExpr(MakeUnique<RefNullExpr>(type));
 }
 
-Result BinaryReaderIR::OnRefIsNullExpr(Type type) {
-  return AppendExpr(MakeUnique<RefIsNullExpr>(type));
+Result BinaryReaderIR::OnRefIsNullExpr() {
+  return AppendExpr(MakeUnique<RefIsNullExpr>());
 }
 
 Result BinaryReaderIR::OnNopExpr() {
diff --git a/src/binary-reader-logging.cc b/src/binary-reader-logging.cc
@@ -790,7 +790,7 @@ DEFINE_INDEX(OnTableSizeExpr)
 DEFINE_INDEX_DESC(OnTableFillExpr, "table index")
 DEFINE_INDEX(OnRefFuncExpr)
 DEFINE_TYPE(OnRefNullExpr)
-DEFINE_TYPE(OnRefIsNullExpr)
+DEFINE0(OnRefIsNullExpr)
 DEFINE0(OnNopExpr)
 DEFINE0(OnRethrowExpr);
 DEFINE_INDEX_DESC(OnReturnCallExpr, "func_index")
diff --git a/src/binary-reader-logging.h b/src/binary-reader-logging.h
@@ -201,7 +201,7 @@ class BinaryReaderLogging : public BinaryReaderDelegate {
   Result OnTableFillExpr(Index table) override;
   Result OnRefFuncExpr(Index index) override;
   Result OnRefNullExpr(Type type) override;
-  Result OnRefIsNullExpr(Type type) override;
+  Result OnRefIsNullExpr() override;
   Result OnNopExpr() override;
   Result OnRethrowExpr() override;
   Result OnReturnCallExpr(Index func_index) override;
diff --git a/src/binary-reader-nop.h b/src/binary-reader-nop.h
@@ -278,7 +278,7 @@ class BinaryReaderNop : public BinaryReaderDelegate {
   Result OnTableFillExpr(Index table_index) override { return Result::Ok; }
   Result OnRefFuncExpr(Index func_index) override { return Result::Ok; }
   Result OnRefNullExpr(Type type) override { return Result::Ok; }
-  Result OnRefIsNullExpr(Type type) override { return Result::Ok; }
+  Result OnRefIsNullExpr() override { return Result::Ok; }
   Result OnNopExpr() override { return Result::Ok; }
   Result OnRethrowExpr() override { return Result::Ok; }
   Result OnReturnCallExpr(Index sig_index) override { return Result::Ok; }
diff --git a/src/binary-reader.cc b/src/binary-reader.cc
@@ -1573,13 +1573,10 @@ Result BinaryReader::ReadFunctionBody(Offset end_offset) {
         break;
       }
 
-      case Opcode::RefIsNull: {
-        Type type;
-        CHECK_RESULT(ReadRefType(&type, "ref.is_null type"));
-        CALLBACK(OnRefIsNullExpr, type);
-        CALLBACK(OnOpcodeType, type);
+      case Opcode::RefIsNull:
+        CALLBACK(OnRefIsNullExpr);
+        CALLBACK(OnOpcodeBare);
         break;
-      }
 
       default:
         return ReportUnexpectedOpcode(opcode);
diff --git a/src/binary-reader.h b/src/binary-reader.h
@@ -267,7 +267,7 @@ class BinaryReaderDelegate {
   virtual Result OnTableFillExpr(Index table_index) = 0;
   virtual Result OnRefFuncExpr(Index func_index) = 0;
   virtual Result OnRefNullExpr(Type type) = 0;
-  virtual Result OnRefIsNullExpr(Type type) = 0;
+  virtual Result OnRefIsNullExpr() = 0;
   virtual Result OnNopExpr() = 0;
   virtual Result OnRethrowExpr() = 0;
   virtual Result OnReturnExpr() = 0;
diff --git a/src/binary-writer.cc b/src/binary-writer.cc
@@ -684,11 +684,9 @@ void BinaryWriter::WriteExpr(const Func* func, const Expr* expr) {
       WriteType(stream_, cast<RefNullExpr>(expr)->type, "ref.null type");
       break;
     }
-    case ExprType::RefIsNull: {
+    case ExprType::RefIsNull:
       WriteOpcode(stream_, Opcode::RefIsNull);
-      WriteType(stream_, cast<RefIsNullExpr>(expr)->type, "ref.is_null type");
       break;
-    }
     case ExprType::Nop:
       WriteOpcode(stream_, Opcode::Nop);
       break;
diff --git a/src/interp/binary-reader-interp.cc b/src/interp/binary-reader-interp.cc
@@ -192,7 +192,7 @@ class BinaryReaderInterp : public BinaryReaderNop {
   Result OnMemorySizeExpr() override;
   Result OnRefFuncExpr(Index func_index) override;
   Result OnRefNullExpr(Type type) override;
-  Result OnRefIsNullExpr(Type type) override;
+  Result OnRefIsNullExpr() override;
   Result OnNopExpr() override;
   Result OnReturnExpr() override;
   Result OnSelectExpr(Type result_type) override;
@@ -1240,8 +1240,8 @@ Result BinaryReaderInterp::OnRefNullExpr(Type type) {
   return Result::Ok;
 }
 
-Result BinaryReaderInterp::OnRefIsNullExpr(Type type) {
-  CHECK_RESULT(validator_.OnRefIsNull(loc, type));
+Result BinaryReaderInterp::OnRefIsNullExpr() {
+  CHECK_RESULT(validator_.OnRefIsNull(loc));
   istream_.Emit(Opcode::RefIsNull);
   return Result::Ok;
 }
diff --git a/src/ir.h b/src/ir.h
@@ -409,7 +409,7 @@ class RefTypeExpr : public ExprMixin<TypeEnum> {
 };
 
 typedef RefTypeExpr<ExprType::RefNull> RefNullExpr;
-typedef RefTypeExpr<ExprType::RefIsNull> RefIsNullExpr;
+typedef ExprMixin<ExprType::RefIsNull> RefIsNullExpr;
 
 template <ExprType TypeEnum>
 class OpcodeExpr : public ExprMixin<TypeEnum> {
diff --git a/src/shared-validator.cc b/src/shared-validator.cc
@@ -968,10 +968,10 @@ Result SharedValidator::OnRefFunc(const Location& loc, Var func_var) {
   return result;
 }
 
-Result SharedValidator::OnRefIsNull(const Location& loc, Type type) {
+Result SharedValidator::OnRefIsNull(const Location& loc) {
   Result result = Result::Ok;
   expr_loc_ = &loc;
-  result |= typechecker_.OnRefIsNullExpr(type);
+  result |= typechecker_.OnRefIsNullExpr();
   return result;
 }
 
diff --git a/src/shared-validator.h b/src/shared-validator.h
@@ -150,7 +150,7 @@ class SharedValidator {
   Result OnMemorySize(const Location&);
   Result OnNop(const Location&);
   Result OnRefFunc(const Location&, Var func_var);
-  Result OnRefIsNull(const Location&, Type type);
+  Result OnRefIsNull(const Location&);
   Result OnRefNull(const Location&, Type type);
   Result OnRethrow(const Location&);
   Result OnReturnCall(const Location&, Var func_var);
diff --git a/src/type-checker.cc b/src/type-checker.cc
@@ -670,8 +670,21 @@ Result TypeChecker::OnRefNullExpr(Type type) {
   return Result::Ok;
 }
 
-Result TypeChecker::OnRefIsNullExpr(Type type) {
-  Result result = PopAndCheck1Type(type, "ref.is_null");
+Result TypeChecker::OnRefIsNullExpr() {
+  Type type;
+  Result result = PeekType(0, &type);
+  if (!type.IsRef()) {
+    TypeVector actual;
+    if (Succeeded(result)) {
+      actual.push_back(type);
+    }
+    std::string message =
+        "type mismatch in ref.is_null, expected reference but got " +
+        TypesToString(actual);
+    PrintError("%s", message.c_str());
+    result = Result::Error;
+  }
+  result |= DropTypes(1);
   PushType(Type::I32);
   return result;
 }
diff --git a/src/type-checker.h b/src/type-checker.h
@@ -110,7 +110,7 @@ class TypeChecker {
   Result OnTableFill(Type elem_type);
   Result OnRefFuncExpr(Index func_index);
   Result OnRefNullExpr(Type type);
-  Result OnRefIsNullExpr(Type type);
+  Result OnRefIsNullExpr();
   Result OnRethrow();
   Result OnReturn();
   Result OnSelect(Type expected);
diff --git a/src/validator.cc b/src/validator.cc
@@ -438,7 +438,7 @@ Result Validator::OnRefNullExpr(RefNullExpr* expr) {
 }
 
 Result Validator::OnRefIsNullExpr(RefIsNullExpr* expr) {
-  result_ |= validator_.OnRefIsNull(expr->loc, expr->type);
+  result_ |= validator_.OnRefIsNull(expr->loc);
   return Result::Ok;
 }
 
diff --git a/src/wast-parser.cc b/src/wast-parser.cc
@@ -1996,13 +1996,10 @@ Result WastParser::ParsePlainInstr(std::unique_ptr<Expr>* out_expr) {
       break;
     }
 
-    case TokenType::RefIsNull: {
+    case TokenType::RefIsNull:
       ErrorUnlessOpcodeEnabled(Consume());
-      Type type;
-      CHECK_RESULT(ParseRefKind(&type));
-      out_expr->reset(new RefIsNullExpr(type, loc));
+      out_expr->reset(new RefIsNullExpr(loc));
       break;
-    }
 
     case TokenType::Throw:
       ErrorUnlessOpcodeEnabled(Consume());
diff --git a/src/wat-writer.cc b/src/wat-writer.cc
@@ -824,7 +824,6 @@ Result WatWriter::ExprVisitorDelegate::OnRefNullExpr(RefNullExpr* expr) {
 
 Result WatWriter::ExprVisitorDelegate::OnRefIsNullExpr(RefIsNullExpr* expr) {
   writer_->WritePutsSpace(Opcode::RefIsNull_Opcode.GetName());
-  writer_->WriteRefKind(expr->type, NextChar::Newline);
   return Result::Ok;
 }
 
diff --git a/test/decompile/basic.txt b/test/decompile/basic.txt
@@ -96,7 +96,7 @@
     set_local 0
     nop
     ref.null func
-    ref.is_null func
+    ref.is_null
     drop
     i32.const 0  ;; fi
     call_indirect
diff --git a/test/dump/reference-types.txt b/test/dump/reference-types.txt
@@ -42,7 +42,7 @@
 
   (func (param externref) (result i32)
     local.get 0
-    ref.is_null extern
+    ref.is_null
   )
 
 
@@ -94,7 +94,7 @@ Code[10]:
  - func[3] size=8
  - func[4] size=9
  - func[5] size=9
- - func[6] size=6
+ - func[6] size=5
  - func[7] size=5
  - func[8] size=5
  - func[9] size=5
@@ -131,15 +131,15 @@ Code Disassembly:
  00007c: 0b                         | end
 00007e func[6]:
  00007f: 20 00                      | local.get 0
- 000081: d1 6f                      | ref.is_null extern
- 000083: 0b                         | end
-000085 func[7]:
- 000086: fc 10 00                   | table.size 0
- 000089: 0b                         | end
-00008b func[8]:
- 00008c: fc 10 01                   | table.size 1
- 00008f: 0b                         | end
-000091 func[9]:
- 000092: fc 10 02                   | table.size 2
- 000095: 0b                         | end
+ 000081: d1                         | ref.is_null
+ 000082: 0b                         | end
+000084 func[7]:
+ 000085: fc 10 00                   | table.size 0
+ 000088: 0b                         | end
+00008a func[8]:
+ 00008b: fc 10 01                   | table.size 1
+ 00008e: 0b                         | end
+000090 func[9]:
+ 000091: fc 10 02                   | table.size 2
+ 000094: 0b                         | end
 ;;; STDOUT ;;)
diff --git a/test/interp/reference-types.txt b/test/interp/reference-types.txt
@@ -18,7 +18,7 @@
 
   (func $ref_is_null_func (export "ref_is_null_func") (result i32)
     global.get $g
-    ref.is_null func
+    ref.is_null
   )
 
   (func $ref_func (export "ref_func") (result funcref)
@@ -35,7 +35,7 @@
     call $table_set
     i32.const 0
     table.get $t_func
-    ref.is_null func
+    ref.is_null
   )
 
   (func $global_set (export "global_set") (result funcref)
diff --git a/test/parse/expr/reference-types.txt b/test/parse/expr/reference-types.txt
@@ -40,7 +40,7 @@
 
   (func (param externref) (result i32)
     local.get 0
-    ref.is_null extern
+    ref.is_null
   )
 
   (func (result funcref)
diff --git a/test/spec/binary.txt b/test/spec/binary.txt
@@ -130,12 +130,9 @@ out/test/spec/binary.wast:696: assert_malformed passed:
 out/test/spec/binary.wast:710: assert_malformed passed:
   000001a: error: unfinished section (expected end: 0x1b)
 out/test/spec/binary.wast:741: assert_malformed passed:
-  error: invalid depth: 11 (max 2)
-  0000024: error: OnBrTableExpr callback failed
-out/test/spec/binary.wast:763: assert_malformed passed:
   error: function type variable out of range: 11 (max 1)
   0000025: error: OnBlockExpr callback failed
-out/test/spec/binary.wast:798: assert_malformed passed:
+out/test/spec/binary.wast:776: assert_malformed passed:
   0000017: error: multiple Start sections
-67/67 tests passed.
+66/66 tests passed.
 ;;; STDOUT ;;)
diff --git a/test/spec/bulk-memory-operations/binary.txt b/test/spec/bulk-memory-operations/binary.txt
@@ -174,12 +174,9 @@ out/test/spec/bulk-memory-operations/binary.wast:1103: assert_malformed passed:
 out/test/spec/bulk-memory-operations/binary.wast:1117: assert_malformed passed:
   000001a: error: unfinished section (expected end: 0x1b)
 out/test/spec/bulk-memory-operations/binary.wast:1148: assert_malformed passed:
-  error: invalid depth: 11 (max 2)
-  0000024: error: OnBrTableExpr callback failed
-out/test/spec/bulk-memory-operations/binary.wast:1170: assert_malformed passed:
   error: function type variable out of range: 11 (max 1)
   0000025: error: OnBlockExpr callback failed
-out/test/spec/bulk-memory-operations/binary.wast:1205: assert_malformed passed:
+out/test/spec/bulk-memory-operations/binary.wast:1183: assert_malformed passed:
   0000017: error: multiple Start sections
-88/88 tests passed.
+87/87 tests passed.
 ;;; STDOUT ;;)
diff --git a/test/spec/reference-types/binary.txt b/test/spec/reference-types/binary.txt
@@ -226,12 +226,9 @@ out/test/spec/reference-types/binary.wast:1452: assert_malformed passed:
 out/test/spec/reference-types/binary.wast:1466: assert_malformed passed:
   000001a: error: unfinished section (expected end: 0x1b)
 out/test/spec/reference-types/binary.wast:1497: assert_malformed passed:
-  error: invalid depth: 11 (max 2)
-  0000024: error: OnBrTableExpr callback failed
-out/test/spec/reference-types/binary.wast:1519: assert_malformed passed:
   error: function type variable out of range: 11 (max 1)
   0000025: error: OnBlockExpr callback failed
-out/test/spec/reference-types/binary.wast:1554: assert_malformed passed:
+out/test/spec/reference-types/binary.wast:1532: assert_malformed passed:
   0000017: error: multiple Start sections
-114/114 tests passed.
+113/113 tests passed.
 ;;; STDOUT ;;)
diff --git a/test/spec/reference-types/ref_is_null.txt b/test/spec/reference-types/ref_is_null.txt
@@ -4,5 +4,11 @@
 (;; STDOUT ;;;
 init(externref:1) =>
 deinit() =>
-13/13 tests passed.
+out/test/spec/reference-types/ref_is_null.wast:52: assert_invalid passed:
+  error: type mismatch in ref.is_null, expected reference but got [i32]
+  000001b: error: OnRefIsNullExpr callback failed
+out/test/spec/reference-types/ref_is_null.wast:56: assert_invalid passed:
+  error: type mismatch in ref.is_null, expected reference but got []
+  0000018: error: OnRefIsNullExpr callback failed
+15/15 tests passed.
 ;;; STDOUT ;;)
diff --git a/test/spec/reference-types/unreached-invalid.txt b/test/spec/reference-types/unreached-invalid.txt
diff --git a/third_party/testsuite b/third_party/testsuite

Original file line number	Diff line number	Diff line change
`@@ -968,10 +968,10 @@ Result SharedValidator::OnRefFunc(const Location& loc, Var func_var) {`
`968`	`968`	`return result;`
`969`	`969`	`}`
`970`	`970`
`971`		`-Result SharedValidator::OnRefIsNull(const Location& loc, Type type) {`
	`971`	`+Result SharedValidator::OnRefIsNull(const Location& loc) {`
`972`	`972`	`Result result = Result::Ok;`
`973`	`973`	`expr_loc_ = &loc;`
`974`		`- result \|= typechecker_.OnRefIsNullExpr(type);`
	`974`	`+ result \|= typechecker_.OnRefIsNullExpr();`
`975`	`975`	`return result;`
`976`	`976`	`}`
`977`	`977`