Revert "Void is not required to be null anymore."

This reverts commit 4e52c45.

BUG=

Review-Url: https://codereview.chromium.org/2863463002 .

CHANGELOG.md

@@ -1,9 +1,6 @@
 ## 1.24.0
 
 ### Language
-* During a dynamic type check, `void` is not required to be `null` anymore.
-  In practice, this makes overriding `void` functions with non-`void` functions
-  safer.
 
 #### Strong Mode
 

pkg/compiler/lib/src/inferrer/type_graph_nodes.dart

@@ -1753,13 +1753,14 @@ TypeMask _narrowType(
     {bool isNullable: true}) {
   if (annotation.treatAsDynamic) return type;
   if (annotation.isObject) return type;
-  if (annotation.isVoid) return type;
   TypeMask otherType;
   if (annotation.isTypedef || annotation.isFunctionType) {
     otherType = closedWorld.commonMasks.functionType;
   } else if (annotation.isTypeVariable) {
     // TODO(ngeoffray): Narrow to bound.
     return type;
+  } else if (annotation.isVoid) {
+    otherType = closedWorld.commonMasks.nullType;
   } else {
     ResolutionInterfaceType interfaceType = annotation;
     otherType = new TypeMask.nonNullSubtype(interfaceType.element, closedWorld);

pkg/compiler/lib/src/inferrer/type_system.dart

@@ -282,7 +282,7 @@ class TypeSystem {
       TypeInformation type, ResolutionDartType annotation,
       {bool isNullable: true}) {
     if (annotation.treatAsDynamic) return type;
-    if (annotation.isVoid) return type;
+    if (annotation.isVoid) return nullType;
     if (annotation.element == closedWorld.commonElements.objectClass &&
         isNullable) {
       return type;

pkg/compiler/lib/src/js_backend/checked_mode_helpers.dart

@@ -117,6 +117,7 @@ class CheckedModeHelpers {
   /// All the checked mode helpers.
   static const List<CheckedModeHelper> helpers = const <CheckedModeHelper>[
     const MalformedCheckedModeHelper('checkMalformedType'),
+    const CheckedModeHelper('voidTypeCheck'),
     const CheckedModeHelper('stringTypeCast'),
     const CheckedModeHelper('stringTypeCheck'),
     const CheckedModeHelper('doubleTypeCast'),
@@ -202,6 +203,12 @@ class CheckedModeHelpers {
       return 'checkMalformedType';
     }
 
+    if (type.isVoid) {
+      assert(!typeCast); // Cannot cast to void.
+      if (nativeCheckOnly) return null;
+      return 'voidTypeCheck';
+    }
+
     if (type.isTypeVariable) {
       return typeCast
           ? 'subtypeOfRuntimeTypeCast'

pkg/compiler/lib/src/js_backend/impact_transformer.dart

@@ -363,7 +363,6 @@ class CodegenImpactTransformer {
 
   void onIsCheckForCodegen(DartType type, TransformedWorldImpact transformed) {
     if (type.isDynamic) return;
-    if (type.isVoid) return;
     type = type.unaliased;
     _impacts.typeCheck.registerImpact(transformed, _elementEnvironment);
 

pkg/compiler/lib/src/ssa/codegen.dart

@@ -2928,7 +2928,6 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
     DartType type = node.typeExpression;
     assert(!type.isTypedef);
     assert(!type.isDynamic);
-    assert(!type.isVoid);
     if (type.isFunctionType) {
       // TODO(5022): We currently generate $isFunction checks for
       // function types.

pkg/compiler/lib/src/ssa/nodes.dart

@@ -1342,9 +1342,8 @@ abstract class HInstruction implements Spannable {
     assert(!type.isTypeVariable);
     assert(type.treatAsRaw || type.isFunctionType);
     if (type.isDynamic) return this;
-    if (type.isVoid) return this;
     if (type == closedWorld.commonElements.objectType) return this;
-    if (type.isFunctionType || type.isMalformed) {
+    if (type.isVoid || type.isFunctionType || type.isMalformed) {
       return new HTypeConversion(
           type, kind, closedWorld.commonMasks.dynamicType, this);
     }

runtime/vm/flow_graph_builder.cc

@@ -1259,9 +1259,8 @@ bool EffectGraphVisitor::CanSkipTypeCheck(TokenPosition token_pos,
     return false;
   }
 
-  // Any type is more specific than the dynamic type, the Object type, or void.
-  if (dst_type.IsDynamicType() || dst_type.IsObjectType() ||
-      dst_type.IsVoidType()) {
+  // Any type is more specific than the dynamic type and than the Object type.
+  if (dst_type.IsDynamicType() || dst_type.IsObjectType()) {
     return true;
   }
 

runtime/vm/flow_graph_compiler_arm.cc

@@ -532,6 +532,11 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
     Label* is_instance_lbl,
     Label* is_not_instance_lbl) {
   __ Comment("InlineInstanceof");
+  if (type.IsVoidType()) {
+    // A non-null value is returned from a void function, which will result in a
+    // type error. A null value is handled prior to executing this inline code.
+    return SubtypeTestCache::null();
+  }
   if (type.IsInstantiated()) {
     const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
     // A class equality check is only applicable with a dst type (not a
@@ -574,7 +579,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
                                            const AbstractType& type,
                                            LocationSummary* locs) {
   ASSERT(type.IsFinalized() && !type.IsMalformed() && !type.IsMalbounded());
-  ASSERT(!type.IsObjectType() && !type.IsDynamicType() && !type.IsVoidType());
+  ASSERT(!type.IsObjectType() && !type.IsDynamicType());
   const Register kInstantiatorTypeArgumentsReg = R2;
   const Register kFunctionTypeArgumentsReg = R1;
   __ PushList((1 << kInstantiatorTypeArgumentsReg) |
@@ -653,8 +658,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   ASSERT(dst_type.IsFinalized());
   // Assignable check is skipped in FlowGraphBuilder, not here.
   ASSERT(dst_type.IsMalformedOrMalbounded() ||
-         (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
-          !dst_type.IsVoidType()));
+         (!dst_type.IsDynamicType() && !dst_type.IsObjectType()));
   const Register kInstantiatorTypeArgumentsReg = R2;
   const Register kFunctionTypeArgumentsReg = R1;
   __ PushList((1 << kInstantiatorTypeArgumentsReg) |

runtime/vm/flow_graph_compiler_arm64.cc

@@ -524,6 +524,11 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
     Label* is_instance_lbl,
     Label* is_not_instance_lbl) {
   __ Comment("InlineInstanceof");
+  if (type.IsVoidType()) {
+    // A non-null value is returned from a void function, which will result in a
+    // type error. A null value is handled prior to executing this inline code.
+    return SubtypeTestCache::null();
+  }
   if (type.IsInstantiated()) {
     const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
     // A class equality check is only applicable with a dst type (not a
@@ -566,7 +571,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
                                            const AbstractType& type,
                                            LocationSummary* locs) {
   ASSERT(type.IsFinalized() && !type.IsMalformed() && !type.IsMalbounded());
-  ASSERT(!type.IsObjectType() && !type.IsDynamicType() && !type.IsVoidType());
+  ASSERT(!type.IsObjectType() && !type.IsDynamicType());
   const Register kInstantiatorTypeArgumentsReg = R1;
   const Register kFunctionTypeArgumentsReg = R2;
   __ PushPair(kFunctionTypeArgumentsReg, kInstantiatorTypeArgumentsReg);
@@ -645,8 +650,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   ASSERT(dst_type.IsFinalized());
   // Assignable check is skipped in FlowGraphBuilder, not here.
   ASSERT(dst_type.IsMalformedOrMalbounded() ||
-         (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
-          !dst_type.IsVoidType()));
+         (!dst_type.IsDynamicType() && !dst_type.IsObjectType()));
   const Register kInstantiatorTypeArgumentsReg = R1;
   const Register kFunctionTypeArgumentsReg = R2;
   __ PushPair(kFunctionTypeArgumentsReg, kInstantiatorTypeArgumentsReg);

runtime/vm/flow_graph_compiler_ia32.cc

@@ -538,6 +538,11 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
     Label* is_instance_lbl,
     Label* is_not_instance_lbl) {
   __ Comment("InlineInstanceof");
+  if (type.IsVoidType()) {
+    // A non-null value is returned from a void function, which will result in a
+    // type error. A null value is handled prior to executing this inline code.
+    return SubtypeTestCache::null();
+  }
   if (type.IsInstantiated()) {
     const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
     // A class equality check is only applicable with a dst type (not a
@@ -580,7 +585,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
                                            const AbstractType& type,
                                            LocationSummary* locs) {
   ASSERT(type.IsFinalized() && !type.IsMalformedOrMalbounded());
-  ASSERT(!type.IsObjectType() && !type.IsDynamicType() && !type.IsVoidType());
+  ASSERT(!type.IsObjectType() && !type.IsDynamicType());
 
   __ pushl(EDX);  // Store instantiator type arguments.
   __ pushl(ECX);  // Store function type arguments.
@@ -661,8 +666,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   ASSERT(dst_type.IsFinalized());
   // Assignable check is skipped in FlowGraphBuilder, not here.
   ASSERT(dst_type.IsMalformedOrMalbounded() ||
-         (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
-          !dst_type.IsVoidType()));
+         (!dst_type.IsDynamicType() && !dst_type.IsObjectType()));
   __ pushl(EDX);  // Store instantiator type arguments.
   __ pushl(ECX);  // Store function type arguments.
   // A null object is always assignable and is returned as result.

runtime/vm/flow_graph_compiler_mips.cc

@@ -520,6 +520,11 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
     Label* is_instance_lbl,
     Label* is_not_instance_lbl) {
   __ Comment("InlineInstanceof");
+  if (type.IsVoidType()) {
+    // A non-null value is returned from a void function, which will result in a
+    // type error. A null value is handled prior to executing this inline code.
+    return SubtypeTestCache::null();
+  }
   if (type.IsInstantiated()) {
     const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
     // A class equality check is only applicable with a dst type (not a
@@ -562,7 +567,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
                                            const AbstractType& type,
                                            LocationSummary* locs) {
   ASSERT(type.IsFinalized() && !type.IsMalformed() && !type.IsMalbounded());
-  ASSERT(!type.IsObjectType() && !type.IsDynamicType() && !type.IsVoidType());
+  ASSERT(!type.IsObjectType() && !type.IsDynamicType());
 
   // Preserve instantiator type arguments (A1) and function type arguments (A2).
   __ addiu(SP, SP, Immediate(-2 * kWordSize));
@@ -648,8 +653,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   ASSERT(dst_type.IsFinalized());
   // Assignable check is skipped in FlowGraphBuilder, not here.
   ASSERT(dst_type.IsMalformedOrMalbounded() ||
-         (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
-          !dst_type.IsVoidType()));
+         (!dst_type.IsDynamicType() && !dst_type.IsObjectType()));
 
   // Preserve instantiator type arguments (A1) and function type arguments (A2).
   __ addiu(SP, SP, Immediate(-2 * kWordSize));

runtime/vm/flow_graph_compiler_x64.cc

@@ -533,6 +533,11 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
     Label* is_instance_lbl,
     Label* is_not_instance_lbl) {
   __ Comment("InlineInstanceof");
+  if (type.IsVoidType()) {
+    // A non-null value is returned from a void function, which will result in a
+    // type error. A null value is handled prior to executing this inline code.
+    return SubtypeTestCache::null();
+  }
   if (type.IsInstantiated()) {
     const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
     // A class equality check is only applicable with a dst type (not a
@@ -575,7 +580,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
                                            const AbstractType& type,
                                            LocationSummary* locs) {
   ASSERT(type.IsFinalized() && !type.IsMalformedOrMalbounded());
-  ASSERT(!type.IsObjectType() && !type.IsDynamicType() && !type.IsVoidType());
+  ASSERT(!type.IsObjectType() && !type.IsDynamicType());
 
   __ pushq(RDX);  // Store instantiator type arguments.
   __ pushq(RCX);  // Store function type arguments.
@@ -584,8 +589,8 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
   // If type is instantiated and non-parameterized, we can inline code
   // checking whether the tested instance is a Smi.
   if (type.IsInstantiated()) {
-    // A null object is only an instance of Null, Object, void and dynamic.
-    // Object void and dynamic have already been checked above (if the type is
+    // A null object is only an instance of Null, Object, and dynamic.
+    // Object and dynamic have already been checked above (if the type is
     // instantiated). So we can return false here if the instance is null,
     // unless the type is Null (and if the type is instantiated).
     // We can only inline this null check if the type is instantiated at compile
@@ -654,8 +659,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
   ASSERT(dst_type.IsFinalized());
   // Assignable check is skipped in FlowGraphBuilder, not here.
   ASSERT(dst_type.IsMalformedOrMalbounded() ||
-         (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
-          !dst_type.IsVoidType()));
+         (!dst_type.IsDynamicType() && !dst_type.IsObjectType()));
   __ pushq(RDX);  // Store instantiator type arguments.
   __ pushq(RCX);  // Store function type arguments.
   // A null object is always assignable and is returned as result.

runtime/vm/flow_graph_type_propagator.cc

@@ -597,7 +597,7 @@ intptr_t CompileType::ToNullableCid() {
     } else if (type_->IsMalformed()) {
       cid_ = kDynamicCid;
     } else if (type_->IsVoidType()) {
-      cid_ = kDynamicCid;
+      cid_ = kNullCid;
     } else if (type_->IsFunctionType() || type_->IsDartFunctionType()) {
       cid_ = kClosureCid;
     } else if (type_->HasResolvedTypeClass()) {
@@ -684,7 +684,7 @@ bool CompileType::CanComputeIsInstanceOf(const AbstractType& type,
     return false;
   }
 
-  if (type.IsDynamicType() || type.IsObjectType() || type.IsVoidType()) {
+  if (type.IsDynamicType() || type.IsObjectType()) {
     *is_instance = true;
     return true;
   }
@@ -696,6 +696,11 @@ bool CompileType::CanComputeIsInstanceOf(const AbstractType& type,
   // Consider the compile type of the value.
   const AbstractType& compile_type = *ToAbstractType();
 
+  // The compile-type of a value should never be void. The result of a void
+  // function must always be null, which was checked to be null at the return
+  // statement inside the function.
+  ASSERT(!compile_type.IsVoidType());
+
   if (compile_type.IsMalformedOrMalbounded()) {
     return false;
   }
@@ -710,6 +715,12 @@ bool CompileType::CanComputeIsInstanceOf(const AbstractType& type,
     return true;
   }
 
+  // A non-null value is not an instance of void.
+  if (type.IsVoidType()) {
+    *is_instance = IsNull();
+    return HasDecidableNullability();
+  }
+
   // If the value can be null then we can't eliminate the
   // check unless null is allowed.
   if (is_nullable_ && !is_nullable) {
@@ -726,6 +737,11 @@ bool CompileType::IsMoreSpecificThan(const AbstractType& other) {
     return false;
   }
 
+  if (other.IsVoidType()) {
+    // The only value assignable to void is null.
+    return IsNull();
+  }
+
   return ToAbstractType()->IsMoreSpecificThan(other, NULL, NULL, Heap::kOld);
 }
 
@@ -927,6 +943,11 @@ CompileType AssertAssignableInstr::ComputeType() const {
     return *value_type;
   }
 
+  if (dst_type().IsVoidType()) {
+    // The only value assignable to void is null.
+    return CompileType::Null();
+  }
+
   return CompileType::Create(value_type->ToCid(), dst_type());
 }
 
@@ -1017,9 +1038,13 @@ CompileType StaticCallInstr::ComputeType() const {
   }
 
   if (Isolate::Current()->type_checks()) {
+    // Void functions are known to return null, which is checked at the return
+    // from the function.
     const AbstractType& result_type =
         AbstractType::ZoneHandle(function().result_type());
-    return CompileType::FromAbstractType(result_type);
+    return CompileType::FromAbstractType(
+        result_type.IsVoidType() ? AbstractType::ZoneHandle(Type::NullType())
+                                 : result_type);
   }
 
   return CompileType::Dynamic();

runtime/vm/object.cc

@@ -15825,7 +15825,7 @@ bool Instance::IsInstanceOf(
   ASSERT(!other.IsMalformed());
   ASSERT(!other.IsMalbounded());
   if (other.IsVoidType()) {
-    return true;
+    return false;
   }
   Zone* zone = Thread::Current()->zone();
   const Class& cls = Class::Handle(zone, clazz());

sdk/lib/_internal/js_runtime/lib/js_helper.dart

@@ -3415,6 +3415,11 @@ listSuperNativeTypeCast(value, property) {
   propertyTypeCastError(value, property);
 }
 
+voidTypeCheck(value) {
+  if (value == null) return value;
+  throw new TypeErrorImplementation(value, 'void');
+}
+
 extractFunctionTypeObjectFrom(o) {
   var interceptor = getInterceptor(o);
   var signatureName = JS_GET_NAME(JsGetName.SIGNATURE_NAME);

tests/co19/co19-co19.status

@@ -66,12 +66,3 @@ LibTest/math/cos_A01_t01: PASS, FAIL, OK  # Issue 26261
 LibTest/math/tan_A01_t01: PASS, FAIL, OK  # Issue 26261
 LibTest/math/log_A01_t01: PASS, FAIL, OK  # Issue 26261
 
-[ $compiler != dart2analyzer && $checked ]
-# Tests that fail on every runtime in checked mode, but not on the analyzer.
-Language/Functions/generator_return_type_t01: Fail # Co19 issue 110
-Language/Functions/generator_return_type_t02: Fail # Co19 issue 110
-Language/Functions/async_return_type_t01: Fail # Co19 issue 110
-Language/Types/Type_Void/returning_t02: Fail # Co19 issue 110
-Language/Types/Type_Void/returning_t03: Fail # Co19 issue 110
-Language/Types/Type_Void/returning_t04: Fail # Co19 issue 110
-Language/Types/Type_Void/returning_t05: Fail # Co19 issue 110

tests/language/return_type_test.dart

@@ -21,7 +21,7 @@ void returnString2() => 's';
 main() {
   if (isCheckedMode()) {
     Expect.throws(returnString1, (e) => e is TypeError);
-    returnString2();
+    Expect.throws(returnString2, (e) => e is TypeError);
     returnNull();
   } else {
     returnString1();