Adding check or trust type checks to builder_kernel.dart.

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

 // Copyright (c) 2016, 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.
 
 import 'package:kernel/ast.dart' as ir;
 
 import '../common.dart';
 import '../common/codegen.dart' show CodegenRegistry, CodegenWorkItem;
 import '../common/names.dart';
 import '../common/tasks.dart' show CompilerTask;
 import '../compiler.dart';
 import '../dart_types.dart';
 import '../elements/elements.dart';
 import '../io/source_information.dart';
 import '../js_backend/backend.dart' show JavaScriptBackend;
 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';
 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;
   final SourceInformationStrategy sourceInformationFactory;
 
   String get name => 'SSA kernel builder';
 
   SsaKernelBuilderTask(JavaScriptBackend backend, this.sourceInformationFactory)
       : backend = backend,
         super(backend.compiler.measurer);
@@ skipping 17 matching lines @@
 
   @override
   JavaScriptBackend get backend => compiler.backend;
 
   @override
   TreeElements get elements => resolvedAst.elements;
 
   SourceInformationBuilder sourceInformationBuilder;
   KernelAstAdapter astAdapter;
   LoopHandler<ir.Node> loopHandler;
+  TypeVerifier typeVerifier;
 
   KernelSsaBuilder(
       this.targetElement,
       this.resolvedAst,
       Compiler compiler,
       this.registry,
       SourceInformationStrategy sourceInformationFactory,
       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 =
         sourceInformationFactory.createBuilderForContext(resolvedAst);
     graph.sourceInformation =
         sourceInformationBuilder.buildVariableDeclaration();
     this.localsHandler = new LocalsHandler(this, targetElement, null, compiler);
     this.astAdapter = new KernelAstAdapter(kernel, compiler.backend,
         resolvedAst, kernel.nodeToAst, kernel.nodeToElement);
     Element originTarget = targetElement;
@@ skipping 26 matching lines @@
     if (field.initializer != null) {
       field.initializer.accept(this);
     } else {
       stack.add(graph.addConstantNull(compiler));
     }
     HInstruction value = pop();
     closeAndGotoExit(new HReturn(value, null));
     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;
   }
 
   void buildConstructor(ir.Constructor constructor) {
     // TODO(het): Actually handle this correctly
     HBasicBlock block = graph.addNewBlock();
     open(graph.entry);
     close(new HGoto()).addSuccessor(block);
     open(block);
     closeAndGotoExit(new HGoto());
     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();
     procedure.function.body.accept(this);
     closeFunction();
   }
 
   void openFunction() {
     HBasicBlock block = graph.addNewBlock();
     open(graph.entry);
     localsHandler.startFunction(targetElement, resolvedAst.node);
     close(new HGoto()).addSuccessor(block);
 
     open(block);
   }
 
   void closeFunction() {
     if (!isAborted()) closeAndGotoExit(new HGoto());
     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());
     for (ir.Statement statement in block.statements) {
       statement.accept(this);
       if (!isReachable) {
         // The block has been aborted by a return or a throw.
         if (stack.isNotEmpty) {
           compiler.reporter.internalError(
               NO_LOCATION_SPANNABLE, 'Non-empty instruction stack.');
@@ skipping 13 matching lines @@
     exprStatement.expression.accept(this);
     pop();
   }
 
   @override
   void visitReturnStatement(ir.ReturnStatement returnStatement) {
     HInstruction value;
     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
     // support inlining.
     closeAndGotoExit(new HReturn(value, null));
   }
 
   @override
   void visitForStatement(ir.ForStatement forStatement) {
     assert(isReachable);
@@ skipping 371 matching lines @@
     staticSet.value.accept(this);
     HInstruction value = pop();
 
     var staticTarget = staticSet.target;
     if (staticTarget is ir.Procedure) {
       // Invoke the setter
       _pushStaticInvocation(staticTarget, <HInstruction>[value],
           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);
   }
 
   @override
   void visitPropertyGet(ir.PropertyGet propertyGet) {
     propertyGet.receiver.accept(this);
     HInstruction receiver = pop();
 
     _pushDynamicInvocation(propertyGet, astAdapter.typeOfGet(propertyGet),
@@ skipping 34 matching lines @@
   void _visitLocalSetter(ir.VariableDeclaration variable, HInstruction value) {
     // TODO(het): handle case where the variable is top-level or static
     LocalElement local = astAdapter.getElement(variable);
 
     // Give the value a name if it doesn't have one already.
     if (value.sourceElement == null) {
       value.sourceElement = local;
     }
 
     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
   /// Extracts the list of instructions for the expressions in the arguments.
   List<HInstruction> _visitArguments(ir.Arguments arguments) {
     List<HInstruction> result = <HInstruction>[];
 
     for (ir.Expression argument in arguments.positional) {
       argument.accept(this);
       result.add(pop());
@@ skipping 155 matching lines @@
     push(new HNot(popBoolified(), backend.boolType));
   }
 
   @override
   void visitStringConcatenation(ir.StringConcatenation stringConcat) {
     KernelStringBuilder stringBuilder = new KernelStringBuilder(this);
     stringConcat.accept(stringBuilder);
     stack.add(stringBuilder.result);
   }
 }