Better runtime type checks.

frog/value.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Represents a meta-value for code generation.
  */
 class Value {
   /** The [Type] of the [Value]. */
   Type type;
@@ skipping 139 matching lines @@
   }
 
   checkFirstClass(SourceSpan span) {
     if (isType) {
       world.error('Types are not first class', span);
     }
   }
 
   /** Generate a call to an unknown function type. */
   Value _varCall(MethodGenerator context, Arguments args) {
+    // TODO(jmesserly): calls to unknown functions will bypass type checks,
+    // which normally happen on the caller side, or in the generated stub for
+    // dynamic method calls. What should we do?

[jimhug, 2011/11/07 16:41:26]

I believe that in checked mode we will have to generate "checking stubs" for
functions that could be used in this situation.  These stubs would perform any
needed type checks and forward on to the actual funciton.

     var stub = world.functionType.getCallStub(args);
     return new Value(null, '$code.${stub.name}(${args.getCode()})');
   }
 
+  /** True if convertTo would generate a conversion. */
+  // TODO(jmesserly): I don't like how this is coupled to convertTo.
+  bool needsConversion(Type toType) {
+    var callMethod = toType.getCallMethod();
+    if (callMethod != null) {
+      int arity = callMethod.parameters.length;
+      var myCall = type.getCallMethod();
+      if (myCall == null || myCall.parameters.length != arity) {
+        return true;
+      }
+    }
+    if (options.enableTypeChecks) {
+      Type fromType = type;
+      if (type.isVar && code != 'null') {
+        fromType = world.objectType;
+      }
+      bool bothNum = type.isNum && toType.isNum;
+      return fromType.isSubtypeOf(toType) || bothNum;
+    }
+    return false;
+  }
+
   /**
    * Assign or convert this value to another type.
    * This is used for converting between function types, and inserting type
    * checks when --enable_type_checks is enabled.
    */
+  // WARNING: this needs to be kept in sync with needsConversion above.
   Value convertTo(MethodGenerator context, Type toType, Node node,
       [bool isDynamic=false]) {
 
-    // Check types if enabled, unless this is a dynamic operation
-    bool checked = options.enableTypeChecks && !isDynamic;
+    // Issue type warnings unless we are processing a dynamic operation.
+    bool checked = !isDynamic;
 
     var callMethod = toType.getCallMethod();
     if (callMethod != null) {
       if (checked && !toType.isAssignable(type)) {
         convertWarning(toType, node);
       }
 
       int arity = callMethod.parameters.length;
       var myCall = type.getCallMethod();
       if (myCall == null || myCall.parameters.length != arity) {
         final stub = world.functionType.getCallStub(new Arguments.bare(arity));
         return new Value(toType, 'to\$${stub.name}($code)');
       }
     }
 
     // Don't add runtime asserts unless we have type checks turned on.
     if (!options.enableTypeChecks) {
       return this;
     }
 
-    if (type.isSubtypeOf(toType)) {
-      return this; // widening conversion
-    } else if (checked && !toType.isSubtypeOf(type)) {
+    // If we're assigning from a var, pretend it's Object for the purpose of
+    // runtime checks.
+
+    // TODO(jmesserly): I'm a little bothered by the fact that we can't call
+    // isSubtypeOf directly. If we tracked null literals as the bottom type,
+    // and then only allowed Dynamic to be bottom for generic type args, I think
+    // we'd get the right behavior from isSubtypeOf.
+    Type fromType = type;
+    if (type.isVar && code != 'null') {
+      fromType = world.objectType;
+    }
+
+    // TODO(jmesserly): remove the special case for "num" when our num handling
+    // is better.
+    bool bothNum = type.isNum && toType.isNum;
+    if (!checked || fromType.isSubtypeOf(toType) || bothNum) {
+      // No checks needed for a widening conversion.
+      return this;
+    }
+
+    if (!toType.isSubtypeOf(type)) {
       // According to the static types, this conversion can't work.
       convertWarning(toType, node);
     }
 
+    // Generate a runtime check
     return _typeAssert(context, toType, node);
   }
 
+  // TODO(jmesserly): this generates an unnecessary check for the 90%
+  // case where the thing passed in was a non-overloaded == or != expression
+  // We'll want to eliminate these, probably by tracking non-null bools in the
+  // type system.
+  Value convertToNonNullBool(MethodGenerator context, Node node) {
+    if (!type.isAssignable(world.boolType)) {
+      convertWarning(world.boolType, node);
+    }
+    if (!options.enableTypeChecks) {
+      return this;
+    } else {
+      // TODO(jmesserly): this is hacky.
+      if (code.startsWith('\$notnull_bool')) {
+        return this;
+      } else {
+        return new Value(world.boolType, '\$notnull_bool($code)');
+      }
+    }
+  }
+
   /**
    * Generates a run time type assertion for the given value. This works like
    * [instanceOf], but it allows null since Dart types are nullable.
    * Also it will throw a TypeError if it gets the wrong type.
    */
-  // TODO(jmesserly): this generated code is too verbose.
   Value _typeAssert(MethodGenerator context, Type toType, Node node) {
     if (toType is ParameterType) {
       ParameterType p = toType;
       toType = p.extendsType;
     }
 
-    // TODO(jmesserly): I don't like the duplication with instanceOf
-    var temp = context.getTemp(this);
-    String testCode;
+    if (toType.isObject || toType.isVar) {
+      world.internalError('We thought ${type.name} is not a subtype of ${toType.name}?');
+    }
+
+    // TODO(jmesserly): better assert for integers?
+    if (toType.isNum) toType = world.numType;
+
+    // Generate a check like these:
+    //   obj && obj.is$TypeName()
+    //   $assert_int(obj)
+    //
+    // We rely on the fact that calling an undefined method produces a JS
+    // TypeError. Alternatively we could define fallbacks on Object that throw.
+    String check;
     if (toType.library.isCore && toType.typeofName != null) {
-      testCode = "typeof(${temp.code}) == '${toType.typeofName}'";
-    } else if (toType.isClass && toType is !ConcreteType) {
-      toType.markUsed();
-      testCode = '${temp.code} instanceof ${toType.jsname}';
+      check = '\$assert_${toType.name}($code)';
+
+      if (toType.typeCheckCode == null) {
+        toType.typeCheckCode = '''
+function \$assert_${toType.name}(x) {
+  if (x == null || typeof(x) == "${toType.typeofName}") return x;
+  throw new TypeError("'" + x + "' is not a ${toType.name}.");
+}''';
+      }
     } else {
       toType.isTested = true;
-      testCode = '${temp.code}.is\$${toType.jsname}';
+
+      // If we track nullability, we could simplify this check.
+      var temp = context.getTemp(this);
+      check = '(${context.assignTemp(temp, this).code} &&';
+      check += ' ${temp.code}.is\$${toType.jsname}())';
+      if (this != temp) context.freeTemp(temp);
     }
-    testCode = '(${context.assignTemp(temp, this).code} == null || $testCode)';
-    var test = new Value(world.boolType, testCode);
 
-    var err = world.corelib.types['TypeError'];
-    world.gen.genMethod(err.members['toString']);
-    var args = new Arguments(null, [temp,
-        new Value(world.stringType, '"${toType.name}"')]);
-    var typeErr = err.getConstructor('').invoke(context, node, null, args);
-
-    var result = new Value(toType, '(${test.code} ? ${temp.code} : '
-        + '\$throw(${typeErr.code}))');
-    if (temp != this) context.freeTemp(temp);
-    return result;
+    return new Value(toType, check);
   }
 
   /**
    * Test to see if value is an instance of this type.
    *
    * - If a primitive type, then uses the JavaScript typeof.
    * - If it's a non-generic class, use instanceof.
    * - Otherwise add a fake member to test for.  This value is generated
    *   as a function so that it can be called for a runtime failure.
    */
@@ skipping 266 matching lines @@
       return 1;
     } else if (name != null && other.name == null) {
       return -1;
     } else if (name != null) {
       return name.compareTo(other.name);
     } else {
       return field.name.compareTo(other.field.name);
     }
   }
 }