Enable full type-checks in checked mode.

sdk/lib/_internal/compiler/implementation/ssa/builder.dart

 // Copyright (c) 2012, 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.
 
 part of ssa;
 
 /**
  * A special element for the extra parameter taken by intercepted
  * methods. We need to override [Element.computeType] because our
  * optimizers may look at its declared type.
@@ skipping 342 matching lines @@
   }
 
   /**
    * Returns an [HInstruction] for the given element. If the element is
    * boxed or stored in a closure then the method generates code to retrieve
    * the value.
    */
   HInstruction readLocal(Element element) {
     if (isAccessedDirectly(element)) {
       if (directLocals[element] == null) {
-        builder.compiler.internalError("Cannot find value $element",
-                                       element: element);
+        if (element.isTypeVariable()) {
+          builder.compiler.internalError(
+              "Runtime type information not available for $element",
+              element: builder.compiler.currentElement);
+        } else {
+          builder.compiler.internalError(
+              "Cannot find value $element",
+              element: element);
+        }
       }
       return directLocals[element];
     } else if (isStoredInClosureField(element)) {
       Element redirect = redirectionMapping[element];
       HInstruction receiver = readLocal(closureData.closureElement);
       HInstruction fieldGet = new HFieldGet(redirect, receiver);
       fieldGet.instructionType = builder.getTypeOfCapturedVariable(element);
       builder.add(fieldGet);
       return fieldGet;
     } else if (isBoxed(element)) {
@@ skipping 714 matching lines @@
     LocalsHandler newLocalsHandler = new LocalsHandler.from(localsHandler);
     newLocalsHandler.closureData =
         compiler.closureToClassMapper.computeClosureToClassMapping(
             function, function.parseNode(compiler), elements);
     int argumentIndex = 0;
     if (isInstanceMember) {
       newLocalsHandler.updateLocal(newLocalsHandler.closureData.thisElement,
                                    compiledArguments[argumentIndex++]);
     }
 
-    FunctionSignature signature = function.computeSignature(compiler);
-    signature.orderedForEachParameter((Element parameter) {
-      HInstruction argument = compiledArguments[argumentIndex++];
-      newLocalsHandler.updateLocal(parameter, argument);
-      potentiallyCheckType(argument, parameter.computeType(compiler));
-    });
-
     if (function.isConstructor()) {
       ClassElement enclosing = function.getEnclosingClass();
       if (backend.needsRti(enclosing)) {
         assert(currentNode is NewExpression);
         InterfaceType type = elements.getType(currentNode);
         Link<DartType> typeVariable = enclosing.typeVariables;
         type.typeArguments.forEach((DartType argument) {
           HInstruction instruction =
               analyzeTypeArgument(argument, currentNode);
           newLocalsHandler.updateLocal(typeVariable.head.element, instruction);
           typeVariable = typeVariable.tail;
         });
         while (!typeVariable.isEmpty) {
           newLocalsHandler.updateLocal(typeVariable.head.element,
                                        graph.addConstantNull(constantSystem));
           typeVariable = typeVariable.tail;
         }
       }
     }
 
+    FunctionSignature signature = function.computeSignature(compiler);

[ngeoffray, 2013/05/13 09:10:59]

Also add a similar comment to line 1675 here.

[karlklose, 2013/05/14 13:49:41]

Done.

+    int index = 0;
+    if (isInstanceMember) index++;

[ngeoffray, 2013/05/13 09:10:59]

Why not using argumentIndex?

[karlklose, 2013/05/14 13:49:41]

Done.

+    signature.orderedForEachParameter((Element parameter) {
+      HInstruction argument = compiledArguments[index++];
+      newLocalsHandler.updateLocal(parameter, argument);
+      potentiallyCheckType(argument, parameter.computeType(compiler));
+    });
+
     // TODO(kasperl): Bad smell. We shouldn't be constructing elements here.
     returnElement = new ElementX(const SourceString("result"),
                                  ElementKind.VARIABLE,
                                  function);
     newLocalsHandler.updateLocal(returnElement,
                                  graph.addConstantNull(constantSystem));
     elements = compiler.enqueuer.resolution.getCachedElements(function);
     assert(elements != null);
     returnType = signature.returnType;
     stack = <HInstruction>[];
@@ skipping 62 matching lines @@
     if (newElements == null) {
       compiler.internalError("Element not resolved: $function");
     }
 
     if (canBeInlined == null) {
       canBeInlined = InlineWeeder.canBeInlined(functionExpression, newElements);
       backend.canBeInlined[function] = canBeInlined;
       if (!canBeInlined) return false;
     }
 
+    // TODO(karlklose): remove this and enable inlining of these methods.

[ngeoffray, 2013/05/13 09:10:59]

Could you also explain why it does not work?

[karlklose, 2013/05/14 13:49:41]

Done.

+    if (compiler.enableTypeAssertions &&
+        element.computeType(compiler).containsTypeVariables) {
+      return false;
+    }
+
     assert(canBeInlined);
     InliningState state = enterInlinedMethod(
         function, selector, argumentsNodes, providedArguments, currentNode);
     inlinedFrom(element, () {
       functionExpression.body.accept(this);
     });
     leaveInlinedMethod(state);
     return true;
   }
 
@@ skipping 432 matching lines @@
   void openFunction(Element element, Expression node) {
     assert(invariant(element, element.isImplementation));
     HBasicBlock block = graph.addNewBlock();
     open(graph.entry);
 
     localsHandler.startFunction(element, node);
     close(new HGoto()).addSuccessor(block);
 
     open(block);
 
+    // Add the type parameters of the class as parameters of this method.  This
+    // must be done before adding the normal parameters, because their types
+    // may contain references to type variables.
+    var enclosing = element.enclosingElement;
+    if ((element.isConstructor() || element.isGenerativeConstructorBody())
+        && backend.needsRti(enclosing)) {
+      enclosing.typeVariables.forEach((TypeVariableType typeVariable) {
+        HParameterValue param = addParameter(typeVariable.element);
+        localsHandler.directLocals[typeVariable.element] = param;
+      });
+    }
+
     if (element is FunctionElement) {
       FunctionElement functionElement = element;
       FunctionSignature signature = functionElement.computeSignature(compiler);
       signature.orderedForEachParameter((Element parameterElement) {
         if (elements.isParameterChecked(parameterElement)) {
           addParameterCheckInstruction(parameterElement);
         }
       });
 
       // Put the type checks in the first successor of the entry,
@@ skipping 16 matching lines @@
             localsHandler.directLocals[parameterElement],
             parameterElement.computeType(compiler));
         localsHandler.directLocals[parameterElement] = newParameter;
       });
 
       returnType = signature.returnType;
     } else {
       // Otherwise it is a lazy initializer which does not have parameters.
       assert(element is VariableElement);
     }
+  }
 
-    // Add the type parameters of the class as parameters of this
-    // method.
-    var enclosing = element.enclosingElement;
-    if ((element.isConstructor() || element.isGenerativeConstructorBody())
-        && backend.needsRti(enclosing)) {
-      enclosing.typeVariables.forEach((TypeVariableType typeVariable) {
-        HParameterValue param = addParameter(typeVariable.element);
-        localsHandler.directLocals[typeVariable.element] = param;
-      });
+  HInstruction buildTypeConversion(Compiler compiler, HInstruction original,
+                                   DartType type, int kind) {

[ngeoffray, 2013/05/13 09:10:59]

Please do not duplicate what's already in convertType. Are there other users of
"converType"? If yes, can you move all the code below to "convertType"?

[karlklose, 2013/05/14 13:49:41]

There are users in the optimizer, but they cannot add checks for type variables
and compound types.

I removed 1727-1736 and called convertType instead.

+    if (type == null) return original;
+    if (identical(type.element, compiler.dynamicClass)) return original;
+    if (identical(type.element, compiler.objectClass)) return original;
+    if (type.isMalformed || (type.kind != TypeKind.INTERFACE &&
+        type.kind != TypeKind.TYPE_VARIABLE)) {
+      return new HTypeConversion(type, kind, HType.UNKNOWN, original);
+    } else if (kind == HTypeConversion.BOOLEAN_CONVERSION_CHECK) {
+      // Boolean conversion checks work on non-nullable booleans.
+      return new HTypeConversion(type, kind, HType.BOOLEAN, original);
+    }
+    if (type.kind == TypeKind.INTERFACE) {
+     HType subtype = new HType.subtype(type, compiler);
+     if (type.isRaw) {
+       return new HTypeConversion(type, kind, subtype, original);
+     }
+     HInstruction representations = buildTypeArgumentRepresentations(type);
+     add(representations);
+     return new HTypeConversion.withTypeRepresentation(type, kind, subtype,
+          original, representations);
+    } else {
+      HType subtype = original.instructionType;
+      assert(type.kind == TypeKind.TYPE_VARIABLE);

[ngeoffray, 2013/05/13 09:10:59]

Move that assert one up.

[karlklose, 2013/05/14 13:49:41]

This assert is obsolete now.

+      HInstruction typeVariable = addTypeVariableReference(type);
+      return new HTypeConversion.withTypeRepresentation(type, kind, subtype,
+          original, typeVariable);
     }
   }
 
-  HInstruction potentiallyCheckType(
-      HInstruction original, DartType type,
+  HInstruction potentiallyCheckType(HInstruction original, DartType type,
       { int kind: HTypeConversion.CHECKED_MODE_CHECK }) {
     if (!compiler.enableTypeAssertions) return original;
-    HInstruction other = original.convertType(compiler, type, kind);
+    HInstruction other =
+        buildTypeConversion(compiler,  original, type, kind);
     if (other != original) add(other);
     return other;
   }
 
   HGraph closeFunction() {
     // TODO(kasperl): Make this goto an implicit return.
     if (!isAborted()) closeAndGotoExit(new HGoto());
     graph.finalize();
     return graph;
   }
@@ skipping 853 matching lines @@
       location = send;
     }
     if (Elements.isStaticOrTopLevelField(element)) {
       if (element.isSetter()) {
         HStatic target = new HStatic(element);
         add(target);
         var instruction = buildInvokeStatic(
             <HInstruction>[target, value], HType.UNKNOWN);
         addWithPosition(instruction, location);
       } else {
-        value = potentiallyCheckType(value, element.computeType(compiler));
+        value =
+            potentiallyCheckType(value, element.computeType(compiler));
         addWithPosition(new HStaticStore(element, value), location);
       }
       stack.add(value);
     } else if (Elements.isErroneousElement(element)) {
       // An erroneous element indicates an unresolved static setter.
       generateThrowNoSuchMethod(
           location,
           getTargetName(element, 'set'),
           argumentNodes: (send == null ? const Link<Node>() : send.arguments));
     } else {
       stack.add(value);
       // If the value does not already have a name, give it here.
       if (value.sourceElement == null) {
         value.sourceElement = element;
       }
-      HInstruction checked = potentiallyCheckType(
-          value, element.computeType(compiler));
+      HInstruction checked =
+          potentiallyCheckType(value, element.computeType(compiler));
       if (!identical(checked, value)) {
         pop();
         stack.add(checked);
       }
       localsHandler.updateLocal(element, checked);
     }
   }
 
   HInstruction invokeInterceptor(Set<ClassElement> intercepted,
                                  HInstruction receiver,
@@ skipping 149 matching lines @@
       isNot = true;
     }
     DartType type = elements.getType(typeAnnotation);
     if (type.isMalformed) {
       String reasons = Types.fetchReasonsFromMalformedType(type);
       if (compiler.enableTypeAssertions) {
         generateMalformedSubtypeError(node, expression, type, reasons);
       } else {
         generateRuntimeError(node, '$type is malformed: $reasons');
       }
-      return;
+    } else {

[ngeoffray, 2013/05/13 09:10:59]

You don't need this change anymore.

[karlklose, 2013/05/14 13:49:41]

I know, but I think it reads better. In particular, the negation logic at the
end is easier to find.

+      HInstruction instruction = buildIsNode(node, type, expression);
+      if (isNot) {
+        add(instruction);
+        instruction = new HNot(instruction);
+      }
+      push(instruction);
     }
+  }
 
-    HInstruction instruction;
+  HInstruction buildIsNode(Node node, DartType type, HInstruction expression) {
     if (type.kind == TypeKind.TYPE_VARIABLE) {
       HInstruction runtimeType = addTypeVariableReference(type);
-      Element helper = backend.getGetObjectIsSubtype();
+      Element helper = backend.getObjectIsSubtype();
       HInstruction helperCall = new HStatic(helper);
       add(helperCall);
       List<HInstruction> inputs = <HInstruction>[helperCall, expression,
                                                  runtimeType];
       HInstruction call = buildInvokeStatic(inputs, HType.BOOLEAN);
       add(call);
-      instruction = new HIs(type, <HInstruction>[expression, call],
-                            HIs.VARIABLE_CHECK);
+      return new HIs(type, <HInstruction>[expression, call],
+                     HIs.VARIABLE_CHECK);
     } else if (RuntimeTypes.hasTypeArguments(type)) {
       Element element = type.element;
       Element helper = backend.getCheckSubtype();
       HInstruction helperCall = new HStatic(helper);
       add(helperCall);
       HInstruction representations =
-        buildTypeArgumentRepresentations(type);
+          buildTypeArgumentRepresentations(type);
       add(representations);
       String operator =
           backend.namer.operatorIs(backend.getImplementationClass(element));
       HInstruction isFieldName = addConstantString(node, operator);
       // TODO(karlklose): use [:null:] for [asField] if [element] does not
       // have a subclass.
       HInstruction asFieldName =
           addConstantString(node, backend.namer.substitutionName(element));
       List<HInstruction> inputs = <HInstruction>[helperCall,
                                                  expression,
                                                  isFieldName,
                                                  representations,
                                                  asFieldName];
       HInstruction call = buildInvokeStatic(inputs, HType.BOOLEAN);
       add(call);
-      instruction = new HIs(type, <HInstruction>[expression, call],
-                            HIs.COMPOUND_CHECK);
+      return
+          new HIs(type, <HInstruction>[expression, call], HIs.COMPOUND_CHECK);
     } else {
-      instruction = new HIs(type, <HInstruction>[expression], HIs.RAW_CHECK);
+      return new HIs(type, <HInstruction>[expression], HIs.RAW_CHECK);
     }
-    if (isNot) {
-      add(instruction);
-      instruction = new HNot(instruction);
-    }
-    push(instruction);
   }
 
   void addDynamicSendArgumentsToList(Send node, List<HInstruction> list) {
     Selector selector = elements.getSelector(node);
     if (selector.namedArgumentCount == 0) {
       addGenericSendArgumentsToList(node.arguments, list);
     } else {
       // Visit positional arguments and add them to the list.
       Link<Node> arguments = node.arguments;
       int positionalArgumentCount = selector.positionalArgumentCount;
@@ skipping 1881 matching lines @@
             // TODO(aprelev@gmail.com): Once old catch syntax is removed
             // "if" condition above and this "else" branch should be deleted as
             // type of declared variable won't matter for the catch
             // condition.
             DartType type = elements.getType(declaration.type);
             if (type == null) {
               compiler.cancel('Catch with unresolved type', node: catchBlock);
             }
             // TODO(karlkose): support type arguments here.
             condition = new HIs(type, <HInstruction>[unwrappedException],
-                                HIs.RAW_CHECK, nullOk: true);
+                                HIs.RAW_CHECK);
             push(condition);
           }
         }
       }
 
       void visitThen() {
         CatchBlock catchBlock = link.head;
         link = link.tail;
 
         if (compiler.enableTypeAssertions) {
@@ skipping 547 matching lines @@
           new HSubGraphBlockInformation(elseBranch.graph));
 
     HBasicBlock conditionStartBlock = conditionBranch.block;
     conditionStartBlock.setBlockFlow(info, joinBlock);
     SubGraph conditionGraph = conditionBranch.graph;
     HIf branch = conditionGraph.end.last;
     assert(branch is HIf);
     branch.blockInformation = conditionStartBlock.blockFlow;
   }
 }