Adding check or trust type checks to builder_kernel.dart.

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

Index: pkg/compiler/lib/src/ssa/builder_kernel.dart
diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart
index 6183f3f0d4b94b20eba12d886b0df92944d3b88a..2dba1604ab35768bc9f524ead2e834c72cced6b6 100644
--- a/pkg/compiler/lib/src/ssa/builder_kernel.dart
+++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart
@@ -17,7 +17,9 @@ import '../kernel/kernel.dart';
 import '../resolution/tree_elements.dart';
 import '../tree/dartstring.dart';
 import '../types/masks.dart';
+import '../universe/call_structure.dart' show CallStructure;
 import '../universe/selector.dart';
+import '../universe/use.dart' show TypeUse;
 import 'graph_builder.dart';
 import 'kernel_ast_adapter.dart';
 import 'kernel_string_builder.dart';
@@ -25,6 +27,7 @@ import 'locals_handler.dart';
 import 'loop_handler.dart';
 import 'nodes.dart';
 import 'ssa_branch_builder.dart';
+import 'type_verifier.dart';
 
 class SsaKernelBuilderTask extends CompilerTask {
   final JavaScriptBackend backend;
@@ -62,6 +65,7 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
   SourceInformationBuilder sourceInformationBuilder;
   KernelAstAdapter astAdapter;
   LoopHandler<ir.Node> loopHandler;
+  TypeVerifier typeVerifier;
 
   KernelSsaBuilder(
       this.targetElement,
@@ -72,6 +76,7 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
       Kernel kernel) {
     this.compiler = compiler;
     this.loopHandler = new KernelLoopHandler(this);
+    typeVerifier = new TypeVerifier(this);
     graph.element = targetElement;
     // TODO(het): Should sourceInformationBuilder be in GraphBuilder?
     this.sourceInformationBuilder =
@@ -118,10 +123,18 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     closeFunction();
   }
 
+
+  /// Pops the most recent instruction from the stack and 'boolifies' it.
+  ///
+  /// Boolification is checking if the value is '=== true'.
   @override
   HInstruction popBoolified() {
     HInstruction value = pop();
-    // TODO(het): add boolean conversion type check
+    if (typeVerifier.checkOrTrustTypes) {
+      return typeVerifier.potentiallyCheckOrTrustType(
+          value, compiler.coreTypes.boolType,
+          kind: HTypeConversion.BOOLEAN_CONVERSION_CHECK);
+    }
     HInstruction result = new HBoolify(value, backend.boolType);
     add(result);
     return result;
@@ -137,6 +150,23 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     graph.finalize();
   }
 
+  HTypeConversion buildFunctionTypeConversion(HInstruction original,
+      DartType type, int kind) {
+      String name =
+          kind == HTypeConversion.CAST_TYPE_CHECK ? '_asCheck' : '_assertCheck';
+
+      List<HInstruction> arguments = <HInstruction>[
+        buildFunctionType(type),
+        original
+      ];
+      _pushDynamicInvocation(null, null, arguments,
+          selector: new Selector.call(
+              new Name(name, astAdapter.jsHelperLibrary),
+              CallStructure.ONE_ARG));
+
+      return new HTypeConversion(type, kind, original.instructionType, pop());
+  }
+
   /// Builds a SSA graph for [procedure].
   void buildProcedure(ir.Procedure procedure) {
     openFunction();
@@ -158,12 +188,23 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     graph.finalize();
   }
 
+  /// Pushes a boolean checking [expression] against null.
+  pushCheckNull(HInstruction expression) {
+    push(new HIdentity(
+        expression, graph.addConstantNull(compiler), null, backend.boolType));
+  }
   @override
   void defaultExpression(ir.Expression expression) {
     // TODO(het): This is only to get tests working
     stack.add(graph.addConstantNull(compiler));
   }
 
+  /// Returns the current source element.
+  ///
+  /// The returned element is a declaration element.
+  @override
+  Element get sourceElement => astAdapter.getElement(target);

[Emily Fortuna, 2016/11/11 23:23:11]

really not sure about this as being what sourceElement was intended to be. lmk
-- all the tests pass..... :-P

[sra1, 2016/11/12 00:31:19]

SourceElement is used to mark HInstructions with the source variable that they
correspond to.  This helps us make up names that are the same as the original
code, and I think it is used in checks (like type assertions) to generate

    function(s) {
      H.checkString(s);
      return s.length;
    }

instead of this uglier code that is closer to the SSA graph:

    function(s) {
      var t1;
      t1 = H.checkString(s);
      return t1.length;
    }

i.e. to detect which different HInstructions can use the same JS variable.


So look at the output of --checked code to verify if it looks like the first
example.
We might generate

     return H.checkString(s).length;

if do something like  return s.length + s.codeUnitAt(0);

[Emily Fortuna, 2016/11/14 17:47:56]

Hmmm what dart code should produce that compiles to that? 

import 'package:args/args.dart';

int foo(String s) {
  return s?.length;
}
main(List<String> arguments) {
  final parser = new ArgParser();
  var argResults = parser.parse(arguments);
  print(foo(argResults.rest));
}

but I wasn't seeing the H.checkString call?

The real question is what is the equivalent in the builder_kernel.dart
environment?

[sra1, 2016/11/14 18:00:20]

"return s.length + s.codeUnitAt(0);" is the suggested Dart code to ensure the
checked result is used twice.

You can call the function twice from main,  foo("Hi") and foo(null).
You can make the function be not inlined by having a second return, or by using
@NoInline(), imported from pkg:expect/expect.dart

[Emily Fortuna, 2016/11/14 19:03:39]

Okay. Code looks like option one, so I think we're good:

H.stringTypeCheck(s);
return s.length + C.JSString_methods.codeUnitAt$1(s, 0);

+
   @override
   void visitBlock(ir.Block block) {
     assert(!isAborted());
@@ -197,9 +238,11 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     if (returnStatement.expression == null) {
       value = graph.addConstantNull(compiler);
     } else {
+      assert(target is ir.Procedure);
       returnStatement.expression.accept(this);
-      value = pop();
-      // TODO(het): Check or trust the type of value
+      value = typeVerifier.potentiallyCheckOrTrustType(
+          pop(),
+          astAdapter.getFunctionReturnType((target as ir.Procedure).function));
     }
     // TODO(het): Add source information
     // TODO(het): Set a return value instead of closing the function when we
@@ -591,8 +634,9 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
           astAdapter.returnTypeOf(staticTarget));
       pop();
     } else {
-      // TODO(het): check or trust type
-      add(new HStaticStore(astAdapter.getMember(staticTarget), value));
+      add(new HStaticStore(astAdapter.getMember(staticTarget),
+            typeVerifier.potentiallyCheckOrTrustType(value,
+              astAdapter.getDartType(staticTarget.setterType))));
     }
     stack.add(value);
   }
@@ -647,8 +691,8 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     }
 
     stack.add(value);
-    // TODO(het): check or trust type
-    localsHandler.updateLocal(local, value);
+    localsHandler.updateLocal(local, typeVerifier.potentiallyCheckOrTrustType(
+          value, astAdapter.getDartType(variable.type)));
   }
 
   // TODO(het): Also extract type arguments