sdk/lib/_internal/compiler/implementation/ssa/builder.dart - Issue 13019003: Enable full type-checks in checked mode.

Unified Diff: sdk/lib/_internal/compiler/implementation/ssa/builder.dart

Issue 13019003: Enable full type-checks in checked mode. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Patch Set: Fix a bug. Created 7 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« sdk/lib/_internal/compiler/implementation/lib/js_rti.dart ('K') | « sdk/lib/_internal/compiler/implementation/lib/js_rti.dart ('k') | sdk/lib/_internal/compiler/implementation/ssa/codegen.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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

diff --git a/sdk/lib/_internal/compiler/implementation/ssa/builder.dart b/sdk/lib/_internal/compiler/implementation/ssa/builder.dart

index 48dd75987ec7b1cb9ee4089227b4d814277a7a11..2512eca78c61e3dea67b0f975db4ce72f6cead8e 100644

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

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

@@ -359,8 +359,15 @@ class LocalsHandler {

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);

ngeoffray 2013/04/15 10:59:14 Thank you :)

+ } else {

+ builder.compiler.internalError(

+ "Cannot find value $element",

+ element: element);

+ }

}

return directLocals[element];

} else if (isStoredInClosureField(element)) {

@@ -926,7 +933,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

assert(!link.isEmpty && link.tail.isEmpty);

visit(link.head);

HInstruction value = pop();

- value = potentiallyCheckType(value, variable.computeType(compiler));

+ value = potentiallyCheckType(node, value, variable.computeType(compiler));

close(new HReturn(value)).addSuccessor(graph.exit);

return closeFunction();

}

@@ -1016,14 +1023,6 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

function, returnElement, returnType, elements, stack, localsHandler);

localsHandler = new LocalsHandler.from(localsHandler);

- FunctionSignature signature = function.computeSignature(compiler);

- int index = 0;

- signature.orderedForEachParameter((Element parameter) {

- HInstruction argument = compiledArguments[index++];

- localsHandler.updateLocal(parameter, argument);

- potentiallyCheckType(argument, parameter.computeType(compiler));

- });

if (function.isConstructor()) {

ClassElement enclosing = function.getEnclosingClass();

if (backend.needsRti(enclosing)) {

@@ -1044,6 +1043,15 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

}

+ FunctionSignature signature = function.computeSignature(compiler);

+ int index = 0;

+ signature.orderedForEachParameter((Element parameter) {

+ HInstruction argument = compiledArguments[index++];

+ localsHandler.updateLocal(parameter, argument);

+ potentiallyCheckType(currentNode, argument,

+ parameter.computeType(compiler));

+ });

// TODO(kasperl): Bad smell. We shouldn't be constructing elements here.

returnElement = new ElementX(const SourceString("result"),

ElementKind.VARIABLE,

@@ -1125,6 +1133,12 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

if (!canBeInlined) return false;

}

+ // TODO(karlklose): remove this and enable inlining of these methods.

+ if (compiler.enableTypeAssertions &&

+ element.computeType(compiler).containsTypeVariables) {

+ return false;

+ }

assert(canBeInlined);

InliningState state = enterInlinedMethod(

function, selector, arguments, currentNode);

@@ -1413,7 +1427,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

List<HInstruction> constructorArguments = <HInstruction>[];

classElement.forEachInstanceField(

(ClassElement enclosingClass, Element member) {

- constructorArguments.add(potentiallyCheckType(

+ constructorArguments.add(potentiallyCheckType(function,

fieldValues[member], member.computeType(compiler)));

includeBackendMembers: true,

@@ -1572,6 +1586,18 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

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);

@@ -1598,6 +1624,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

}

HInstruction newParameter = potentiallyCheckType(

+ node,

localsHandler.directLocals[parameterElement],

parameterElement.computeType(compiler));

localsHandler.directLocals[parameterElement] = newParameter;

@@ -1608,26 +1635,26 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

// 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 potentiallyCheckType(

- HInstruction original, DartType type,

+ Node node, HInstruction original, DartType type,

{ int kind: HTypeConversion.CHECKED_MODE_CHECK }) {

if (!compiler.enableTypeAssertions) return original;

- HInstruction other = original.convertType(compiler, type, kind);

- if (other != original) add(other);

- return other;

+ if (type.isDynamic || type.element == compiler.objectClass) {

+ return original;

+ }

+ if (kind == HTypeConversion.CHECKED_MODE_CHECK &&

+ !type.isMalformed) {

+ HInstruction checked =

+ buildIsNode(node, type, original, checkedModeTest: true);

+ add(checked);

+ return checked;

+ } else {

ngeoffray 2013/04/15 10:59:14 You should remove this else. A checked mode check

+ HInstruction other = original.convertType(compiler, type, kind);

+ if (other != original) add(other);

+ return other;

+ }

}

HGraph closeFunction() {

@@ -1702,10 +1729,11 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

stack.add(stack.last);

}

- HInstruction popBoolified() {

+ HInstruction popBoolified(Node node) {

HInstruction value = pop();

if (compiler.enableTypeAssertions) {

return potentiallyCheckType(

+ node,

value,

compiler.boolClass.computeType(compiler),

kind: HTypeConversion.BOOLEAN_CONVERSION_CHECK);

@@ -2063,7 +2091,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

return graph.addConstantBool(true, constantSystem);

}

visit(node.condition);

- return popBoolified();

+ return popBoolified(node);

}

void buildUpdate() {

for (Expression expression in node.update) {

@@ -2083,7 +2111,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

visitWhile(While node) {

HInstruction buildCondition() {

visit(node.condition);

- return popBoolified();

+ return popBoolified(node);

}

handleLoop(node,

() {},

@@ -2163,7 +2191,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

visit(node.condition);

assert(!isAborted());

- HInstruction conditionInstruction = popBoolified();

+ HInstruction conditionInstruction = popBoolified(node);

HBasicBlock conditionEndBlock = close(

new HLoopBranch(conditionInstruction, HLoopBranch.DO_WHILE_LOOP));

@@ -2278,12 +2306,14 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

void handleIf(Node diagnosticNode,

void visitCondition(), void visitThen(), void visitElse()) {

SsaBranchBuilder branchBuilder = new SsaBranchBuilder(this, diagnosticNode);

- branchBuilder.handleIf(visitCondition, visitThen, visitElse);

+ branchBuilder.handleIf(diagnosticNode, visitCondition, visitThen,

+ visitElse);

}

void visitLogicalAndOr(Send node, Operator op) {

SsaBranchBuilder branchBuilder = new SsaBranchBuilder(this, node);

branchBuilder.handleLogicalAndOrWithLeftNode(

+ node,

node.receiver,

() { visit(node.argumentsNode); },

isAnd: (const SourceString("&&") == op.source));

@@ -2292,7 +2322,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

void visitLogicalNot(Send node) {

assert(node.argumentsNode is Prefix);

visit(node.receiver);

- HNot not = new HNot(popBoolified());

+ HNot not = new HNot(popBoolified(node));

pushWithPosition(not, node);

}

@@ -2348,7 +2378,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

buildInvokeDynamic(send, selector, left, [right]),

op);

if (op.source.stringValue == '!=') {

- pushWithPosition(new HNot(popBoolified()), op);

+ pushWithPosition(new HNot(popBoolified(send)), op);

}

@@ -2490,7 +2520,8 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

new HInvokeStatic(<HInstruction>[target, value], HType.UNKNOWN),

send);

} else {

- value = potentiallyCheckType(value, element.computeType(compiler));

+ value =

+ potentiallyCheckType(send, value, element.computeType(compiler));

addWithPosition(new HStaticStore(element, value), send);

}

stack.add(value);

@@ -2505,8 +2536,8 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

if (value.sourceElement == null) {

value.sourceElement = element;

}

- HInstruction checked = potentiallyCheckType(

- value, element.computeType(compiler));

+ HInstruction checked =

+ potentiallyCheckType(send, value, element.computeType(compiler));

if (!identical(checked, value)) {

pop();

stack.add(checked);

@@ -2682,33 +2713,45 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

} else {

generateRuntimeError(node, '$type is malformed: $reasons');

}

- return;

+ } else {

+ 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,

+ {bool checkedModeTest: false}) {

ngeoffray 2013/04/15 10:59:14 I'd prefer if the call sites always added the name

+ HType returnType =

+ checkedModeTest ? expression.instructionType : HType.BOOLEAN;

if (type.kind == TypeKind.TYPE_VARIABLE) {

HInstruction runtimeType = addTypeVariableReference(type);

- Element helper = backend.getGetObjectIsSubtype();

+ Element helper = checkedModeTest ? backend.getAssertObjectIsSubtype()

+ : backend.getObjectIsSubtype();

HInstruction helperCall = new HStatic(helper);

add(helperCall);

List<HInstruction> inputs = <HInstruction>[helperCall, expression,

runtimeType];

- HInstruction call = new HInvokeStatic(inputs, HType.BOOLEAN);

+ HInstruction call = new HInvokeStatic(inputs, returnType);

add(call);

- instruction = new HIs(type, <HInstruction>[expression, call],

- HIs.VARIABLE_CHECK);

+ return new HIs(type, <HInstruction>[expression, call],

+ HIs.VARIABLE_CHECK, checkedModeTest, returnType);

} else if (RuntimeTypes.hasTypeArguments(type)) {

Element element = type.element;

bool needsNativeCheck =

- backend.emitter.nativeEmitter.requiresNativeIsCheck(element);

- Element helper = backend.getCheckSubtype();

+ backend.emitter.nativeEmitter.requiresNativeIsCheck(element);

+ Element helper = checkedModeTest ? backend.getAssertSubtype()

+ : backend.getCheckSubtype();

HInstruction helperCall = new HStatic(helper);

add(helperCall);

HInstruction representations =

- buildTypeArgumentRepresentations(type);

+ buildTypeArgumentRepresentations(type);

add(representations);

HInstruction isFieldName =

- addConstantString(node, backend.namer.operatorIs(element));

+ addConstantString(node, backend.namer.operatorIs(element));

// TODO(karlklose): use [:null:] for [asField] if [element] does not

// have a subclass.

HInstruction asFieldName =

@@ -2721,18 +2764,30 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

representations,

asFieldName,

native];

- HInstruction call = new HInvokeStatic(inputs, HType.BOOLEAN);

+ HInstruction call = new HInvokeStatic(inputs, returnType);

+ add(call);

+ return new HIs(type, <HInstruction>[expression, call],

+ HIs.COMPOUND_CHECK, checkedModeTest, returnType);

+ } else if (checkedModeTest) {

+ FunctionElement helper = backend.getCheckedModeHelper(type);

+ HInstruction helperCall = new HStatic(helper);

+ add(helperCall);

+ List<HInstruction> inputs = <HInstruction>[helperCall, expression];

+ if (helper.functionSignature.requiredParameterCount == 2) {

+ // 2 arguments implies that the method is either [propertyTypeCheck]

+ // or [propertyTypeCast].

+ String isFieldName = backend.namer.operatorIs(type.element);

+ inputs.add(addConstantString(node, isFieldName));

+ }

+ HInstruction call = new HInvokeStatic(inputs, returnType);

add(call);

- instruction = new HIs(type, <HInstruction>[expression, call],

- HIs.COMPOUND_CHECK);

+ return new HIs(type, <HInstruction>[expression, call], HIs.RAW_ASSERT,

+ checkedModeTest, returnType);

} else {

- instruction = new HIs(type, <HInstruction>[expression], HIs.RAW_CHECK);

- }

- if (isNot) {

- add(instruction);

- instruction = new HNot(instruction);

+ assert(!checkedModeTest);

+ return new HIs(type, <HInstruction>[expression], HIs.RAW_CHECK, false,

+ returnType);

}

- push(instruction);

}

void addDynamicSendArgumentsToList(Send node, List<HInstruction> list) {

@@ -3879,7 +3934,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

} else {

visit(node.expression);

value = pop();

- value = potentiallyCheckType(value, returnType);

+ value = potentiallyCheckType(node, value, returnType);

}

handleInTryStatement();

@@ -3947,7 +4002,8 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

visitConditional(Conditional node) {

SsaBranchBuilder brancher = new SsaBranchBuilder(this, node);

- brancher.handleConditional(() => visit(node.condition),

+ brancher.handleConditional(node,

+ () => visit(node.condition),

() => visit(node.thenExpression),

() => visit(node.elseExpression));

}

@@ -4050,7 +4106,7 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

HInstruction buildCondition() {

Selector selector = elements.getMoveNextSelector(node);

push(new HInvokeDynamicMethod(selector, <HInstruction>[iterator]));

- return popBoolified();

+ return popBoolified(node);

}

void buildBody() {

Selector call = elements.getCurrentSelector(node);

@@ -4486,7 +4542,8 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

}

SsaBranchBuilder branchBuilder =

new SsaBranchBuilder(this, remainingCases.head);

- branchBuilder.handleLogicalAndOr(left, right, isAnd: false);

+ branchBuilder.handleLogicalAndOr(remainingCases.head, left, right,

+ isAnd: false);

}

if (node.isDefaultCase) {

@@ -4587,7 +4644,9 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

// TODO(karlkose): support type arguments here.

HInstruction condition = new HIs(type,

<HInstruction>[unwrappedException],

- HIs.RAW_CHECK);

+ HIs.RAW_CHECK,

+ false,

+ HType.BOOLEAN);

push(condition);

}

} else {

@@ -4607,7 +4666,8 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {

}

// TODO(karlkose): support type arguments here.

condition = new HIs(type, <HInstruction>[unwrappedException],

- HIs.RAW_CHECK, nullOk: true);

+ HIs.RAW_CHECK, false, HType.BOOLEAN,

+ nullOk: true);

push(condition);

}

@@ -4958,7 +5018,8 @@ class SsaBranchBuilder {

}

- void buildCondition(void visitCondition(),

+ void buildCondition(Node node,

+ void visitCondition(),

SsaBranch conditionBranch,

SsaBranch thenBranch,

SsaBranch elseBranch) {

@@ -4966,7 +5027,7 @@ class SsaBranchBuilder {

visitCondition();

checkNotAborted();

assert(identical(builder.current, builder.lastOpenedBlock));

- HInstruction conditionValue = builder.popBoolified();

+ HInstruction conditionValue = builder.popBoolified(node);

HIf branch = new HIf(conditionValue);

HBasicBlock conditionExitBlock = builder.current;

builder.close(branch);

@@ -5028,7 +5089,8 @@ class SsaBranchBuilder {

return null;

}

- handleIf(void visitCondition(), void visitThen(), void visitElse()) {

+ handleIf(Node node, void visitCondition(), void visitThen(),

+ void visitElse()) {

if (visitElse == null) {

// Make sure to have an else part to avoid a critical edge. A

// critical edge is an edge that connects a block with multiple

@@ -5038,15 +5100,16 @@ class SsaBranchBuilder {

visitElse = () {};

}

- _handleDiamondBranch(visitCondition, visitThen, visitElse, false);

+ _handleDiamondBranch(node, visitCondition, visitThen, visitElse, false);

}

- handleConditional(void visitCondition(), void visitThen(), void visitElse()) {

+ handleConditional(Node node, void visitCondition(), void visitThen(),

+ void visitElse()) {

assert(visitElse != null);

- _handleDiamondBranch(visitCondition, visitThen, visitElse, true);

+ _handleDiamondBranch(node, visitCondition, visitThen, visitElse, true);

}

- void handleLogicalAndOr(void left(), void right(), {bool isAnd}) {

+ void handleLogicalAndOr(Node node, void left(), void right(), {bool isAnd}) {

// x && y is transformed into:

// t0 = boolify(x);

// if (t0) {

@@ -5065,7 +5128,7 @@ class SsaBranchBuilder {

void visitCondition() {

left();

- boolifiedLeft = builder.popBoolified();

+ boolifiedLeft = builder.popBoolified(node);

builder.stack.add(boolifiedLeft);

if (!isAnd) {

builder.push(new HNot(builder.pop()));

@@ -5074,10 +5137,10 @@ class SsaBranchBuilder {

void visitThen() {

right();

- boolifiedRight = builder.popBoolified();

+ boolifiedRight = builder.popBoolified(node);

}

- handleIf(visitCondition, visitThen, null);

+ handleIf(node, visitCondition, visitThen, null);

HConstant notIsAnd =

builder.graph.addConstantBool(!isAnd, builder.constantSystem);

HPhi result = new HPhi.manyInputs(null,

@@ -5086,7 +5149,8 @@ class SsaBranchBuilder {

builder.stack.add(result);

}

- void handleLogicalAndOrWithLeftNode(Node left,

+ void handleLogicalAndOrWithLeftNode(Node node,

+ Node left,

void visitRight(),

{bool isAnd}) {

// This method is similar to [handleLogicalAndOr] but optimizes the case

@@ -5111,16 +5175,19 @@ class SsaBranchBuilder {

assert(link.tail.isEmpty);

Node middle = link.head;

handleLogicalAndOrWithLeftNode(

+ node,

newLeft,

- () => handleLogicalAndOrWithLeftNode(middle, visitRight,

+ () => handleLogicalAndOrWithLeftNode(node, middle, visitRight,

isAnd: isAnd),

isAnd: isAnd);

} else {

- handleLogicalAndOr(() => builder.visit(left), visitRight, isAnd: isAnd);

+ handleLogicalAndOr(node, () => builder.visit(left), visitRight,

+ isAnd: isAnd);

}

- void _handleDiamondBranch(void visitCondition(),

+ void _handleDiamondBranch(Node node,

+ void visitCondition(),

void visitThen(),

void visitElse(),

bool isExpression) {

@@ -5132,7 +5199,8 @@ class SsaBranchBuilder {

conditionBranch.startLocals = builder.localsHandler;

builder.goto(builder.current, conditionBranch.block);

- buildCondition(visitCondition, conditionBranch, thenBranch, elseBranch);

+ buildCondition(node, visitCondition, conditionBranch, thenBranch,

+ elseBranch);

HInstruction thenValue =

buildBranch(thenBranch, visitThen, joinBranch, isExpression);

HInstruction elseValue =