Skip to content

Commit

Permalink
fixes #192, encode dynamic info in checker
Browse files Browse the repository at this point in the history 
fixes a checker bug too: it wasn't working properly for cascade targets. Previously this caught #202 and #206 as well.

R=leafp@google.com

Review URL: https://codereview.chromium.org/1184843002.
  • Loading branch information
John Messerly committed Jun 12, 2015
1 parent b9c83e0 commit 86c5ee9
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 28 deletions.
53 changes: 40 additions & 13 deletions pkg/dev_compiler/lib/src/checker/checker.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,35 @@ class CodeChecker extends RecursiveAstVisitor {
if (_rules.isDynamicCall(f)) {
// If f is Function and this is a method invocation, we should have
// gotten an analyzer error, so no need to issue another error.
_recordDynamicInvoke(node);
_recordDynamicInvoke(node, f);
} else {
checkArgumentList(list, _rules.getTypeAsCaller(f));
}
}

@override
void visitMethodInvocation(MethodInvocation node) {
checkFunctionApplication(node, node.methodName, node.argumentList);
visitMethodInvocation(MethodInvocation node) {
var target = node.realTarget;
if (_rules.isDynamicTarget(target)) {
_recordDynamicInvoke(node, target);

// Mark the tear-off as being dynamic, too. This lets us distinguish
// cases like:
//
// dynamic d;
// d.someMethod(...); // the whole method call must be a dynamic send.
//
// ... from case like:
//
// SomeType s;
// s.someDynamicField(...); // static get, followed by dynamic call.
//
// The first case is handled here, the second case is handled below when
// we call [checkFunctionApplication].
DynamicInvoke.set(node.methodName, true);
} else {
checkFunctionApplication(node, node.methodName, node.argumentList);
}
node.visitChildren(this);
}

Expand Down Expand Up @@ -622,8 +642,9 @@ class CodeChecker extends RecursiveAstVisitor {

@override
void visitPropertyAccess(PropertyAccess node) {
if (node.staticType.isDynamic && _rules.isDynamicTarget(node.realTarget)) {
_recordDynamicInvoke(node);
var target = node.realTarget;
if (_rules.isDynamicTarget(target)) {
_recordDynamicInvoke(node, target);
}
node.visitChildren(this);
}
Expand All @@ -632,7 +653,7 @@ class CodeChecker extends RecursiveAstVisitor {
void visitPrefixedIdentifier(PrefixedIdentifier node) {
final target = node.prefix;
if (_rules.isDynamicTarget(target)) {
_recordDynamicInvoke(node);
_recordDynamicInvoke(node, target);
}
node.visitChildren(this);
}
Expand Down Expand Up @@ -774,7 +795,7 @@ class CodeChecker extends RecursiveAstVisitor {
op.type == TokenType.PLUS_PLUS ||
op.type == TokenType.MINUS_MINUS) {
if (_rules.isDynamicTarget(node.operand)) {
_recordDynamicInvoke(node);
_recordDynamicInvoke(node, node.operand);
}
// For ++ and --, even if it is not dynamic, we still need to check
// that the user defined method accepts an `int` as the RHS.
Expand All @@ -790,7 +811,7 @@ class CodeChecker extends RecursiveAstVisitor {
// Dynamic invocation
// TODO(vsm): Move this logic to the resolver?
if (op.type != TokenType.EQ_EQ && op.type != TokenType.BANG_EQ) {
_recordDynamicInvoke(node);
_recordDynamicInvoke(node, node.leftOperand);
}
} else {
var element = node.staticElement;
Expand Down Expand Up @@ -831,8 +852,9 @@ class CodeChecker extends RecursiveAstVisitor {

@override
void visitIndexExpression(IndexExpression node) {
if (_rules.isDynamicTarget(node.target)) {
_recordDynamicInvoke(node);
var target = node.realTarget;
if (_rules.isDynamicTarget(target)) {
_recordDynamicInvoke(node, target);
} else {
var element = node.staticElement;
if (element is MethodElement) {
Expand Down Expand Up @@ -897,7 +919,7 @@ class CodeChecker extends RecursiveAstVisitor {
var methodElement = expr.staticElement;
if (methodElement == null) {
// Dynamic invocation
_recordDynamicInvoke(expr);
_recordDynamicInvoke(expr, expr.leftHandSide);
} else {
// Sanity check the operator
assert(methodElement.isOperator);
Expand Down Expand Up @@ -941,8 +963,13 @@ class CodeChecker extends RecursiveAstVisitor {
}
}

void _recordDynamicInvoke(AstNode node) {
_reporter.log(new DynamicInvoke(_rules, node));
void _recordDynamicInvoke(AstNode node, AstNode target) {
var dinvoke = new DynamicInvoke(_rules, node);
_reporter.log(dinvoke);
// TODO(jmesserly): we may eventually want to record if the whole operation
// (node) was dynamic, rather than the target, but this is an easier fit
// with what we used to do.
DynamicInvoke.set(target, true);
}

void _recordMessage(StaticInfo info) {
Expand Down
34 changes: 20 additions & 14 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
id = lhs.identifier;
}

if (target != null && rules.isDynamicTarget(target)) {
if (target != null && DynamicInvoke.get(target)) {
return js.call('dart.$DPUT(#, #, #)', [
_visit(target),
_emitMemberName(id.name, type: getStaticType(target)),
Expand Down Expand Up @@ -1525,7 +1525,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {

String code;
if (target == null || isLibraryPrefix(target)) {
if (rules.isDynamicCall(node.methodName)) {
if (DynamicInvoke.get(node.methodName)) {
code = 'dart.$DCALL(#, #)';
} else {
code = '#(#)';
Expand All @@ -1540,9 +1540,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
bool isStatic = element is ExecutableElement && element.isStatic;
var memberName = _emitMemberName(name, type: type, isStatic: isStatic);

if (rules.isDynamicTarget(target)) {
if (DynamicInvoke.get(target)) {
code = 'dart.$DSEND(#, #, #)';
} else if (rules.isDynamicCall(node.methodName)) {
} else if (DynamicInvoke.get(node.methodName)) {
// This is a dynamic call to a statically know target. For example:
// class Foo { Function bar; }
// new Foo().bar(); // dynamic call
Expand Down Expand Up @@ -1584,7 +1584,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
JS.Expression visitFunctionExpressionInvocation(
FunctionExpressionInvocation node) {
var code;
if (rules.isDynamicCall(node.function)) {
if (DynamicInvoke.get(node.function)) {
code = 'dart.$DCALL(#, #)';
} else {
code = '#(#)';
Expand Down Expand Up @@ -1995,6 +1995,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
new SimpleIdentifier(new StringToken(TokenType.IDENTIFIER, name, -1));
id.staticElement = new TemporaryVariableElement.forNode(id);
id.staticType = type;
DynamicInvoke.set(id, type.isDynamic);
return id;
}

Expand Down Expand Up @@ -2022,25 +2023,30 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
/// needed.
Expression _bindLeftHandSide(
Map<String, JS.Expression> scope, Expression expr, {Expression context}) {
Expression result;
if (expr is IndexExpression) {
IndexExpression index = expr;
return new IndexExpression.forTarget(
result = new IndexExpression.forTarget(
_bindValue(scope, 'o', index.target, context: context),
index.leftBracket,
_bindValue(scope, 'i', index.index, context: context),
index.rightBracket)..staticType = expr.staticType;
index.rightBracket);
} else if (expr is PropertyAccess) {
PropertyAccess prop = expr;
return new PropertyAccess(
result = new PropertyAccess(
_bindValue(scope, 'o', _getTarget(prop), context: context),
prop.operator, prop.propertyName)..staticType = expr.staticType;
prop.operator, prop.propertyName);
} else if (expr is PrefixedIdentifier) {
PrefixedIdentifier ident = expr;
return new PrefixedIdentifier(
result = new PrefixedIdentifier(
_bindValue(scope, 'o', ident.prefix, context: context), ident.period,
ident.identifier)..staticType = expr.staticType;
ident.identifier);
} else {
return expr as SimpleIdentifier;
}
return expr as SimpleIdentifier;
result.staticType = expr.staticType;
DynamicInvoke.set(result, DynamicInvoke.get(expr));
return result;
}

/// Creates a temporary to contain the value of [expr]. The temporary can be
Expand Down Expand Up @@ -2216,7 +2222,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
}
var name = _emitMemberName(memberId.name,
type: getStaticType(target), isStatic: isStatic);
if (rules.isDynamicTarget(target)) {
if (DynamicInvoke.get(target)) {
return js.call('dart.$DLOAD(#, #)', [_visit(target), name]);
}

Expand Down Expand Up @@ -2246,7 +2252,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
Expression target, String name, List<Expression> args) {
var type = getStaticType(target);
var memberName = _emitMemberName(name, unary: args.isEmpty, type: type);
if (rules.isDynamicTarget(target)) {
if (DynamicInvoke.get(target)) {
// dynamic dispatch
var dynamicHelper = const {'[]': DINDEX, '[]=': DSETINDEX}[name];
if (dynamicHelper != null) {
Expand Down
18 changes: 17 additions & 1 deletion pkg/dev_compiler/lib/src/info.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ abstract class CoercionInfo extends StaticInfo {

String get description => '${this.runtimeType}: $baseType to $convertedType';

static const String _propertyName = 'dev_compiler.Conversion';
static const String _propertyName = 'dev_compiler.src.info.CoercionInfo';

/// Gets the coercion info associated with this node.
static CoercionInfo get(AstNode node) => node.getProperty(_propertyName);
Expand Down Expand Up @@ -345,6 +345,22 @@ class DynamicInvoke extends CoercionInfo {
DartType get convertedType => rules.provider.dynamicType;
String get message => '$node requires dynamic invoke';
Level get level => Level.INFO;

static const String _propertyName = 'dev_compiler.src.info.DynamicInvoke';

/// Whether this [node] is the target of a dynamic operation.
static bool get(AstNode node) {
var value = node.getProperty(_propertyName);
return value != null ? value : false;
}

/// Sets whether this node is the target of a dynamic operation.
static bool set(AstNode node, bool value) {
// Free the storage for things that aren't dynamic.
if (value == false) value = null;
node.setProperty(_propertyName, value);
return value;
}
}

abstract class StaticError extends StaticInfo {
Expand Down

0 comments on commit 86c5ee9

Please sign in to comment.