Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit b0e5afe

Browse files
author
John Messerly
committed
fixes #192, encode dynamic info in checker
fixes a checker bug too: it wasn't working properly for cascade targets. Previously this caught #202 and #206 as well. [email protected] Review URL: https://codereview.chromium.org/1184843002.
1 parent 5ac465c commit b0e5afe

File tree

3 files changed

+77
-28
lines changed

3 files changed

+77
-28
lines changed

‎lib/src/checker/checker.dart

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -545,15 +545,35 @@ class CodeChecker extends RecursiveAstVisitor {
545545
if (_rules.isDynamicCall(f)) {
546546
// If f is Function and this is a method invocation, we should have
547547
// gotten an analyzer error, so no need to issue another error.
548-
_recordDynamicInvoke(node);
548+
_recordDynamicInvoke(node, f);
549549
} else {
550550
checkArgumentList(list, _rules.getTypeAsCaller(f));
551551
}
552552
}
553553

554554
@override
555-
void visitMethodInvocation(MethodInvocation node) {
556-
checkFunctionApplication(node, node.methodName, node.argumentList);
555+
visitMethodInvocation(MethodInvocation node) {
556+
var target = node.realTarget;
557+
if (_rules.isDynamicTarget(target)) {
558+
_recordDynamicInvoke(node, target);
559+
560+
// Mark the tear-off as being dynamic, too. This lets us distinguish
561+
// cases like:
562+
//
563+
// dynamic d;
564+
// d.someMethod(...); // the whole method call must be a dynamic send.
565+
//
566+
// ... from case like:
567+
//
568+
// SomeType s;
569+
// s.someDynamicField(...); // static get, followed by dynamic call.
570+
//
571+
// The first case is handled here, the second case is handled below when
572+
// we call [checkFunctionApplication].
573+
DynamicInvoke.set(node.methodName, true);
574+
} else {
575+
checkFunctionApplication(node, node.methodName, node.argumentList);
576+
}
557577
node.visitChildren(this);
558578
}
559579

@@ -622,8 +642,9 @@ class CodeChecker extends RecursiveAstVisitor {
622642

623643
@override
624644
void visitPropertyAccess(PropertyAccess node) {
625-
if (node.staticType.isDynamic && _rules.isDynamicTarget(node.realTarget)) {
626-
_recordDynamicInvoke(node);
645+
var target = node.realTarget;
646+
if (_rules.isDynamicTarget(target)) {
647+
_recordDynamicInvoke(node, target);
627648
}
628649
node.visitChildren(this);
629650
}
@@ -632,7 +653,7 @@ class CodeChecker extends RecursiveAstVisitor {
632653
void visitPrefixedIdentifier(PrefixedIdentifier node) {
633654
final target = node.prefix;
634655
if (_rules.isDynamicTarget(target)) {
635-
_recordDynamicInvoke(node);
656+
_recordDynamicInvoke(node, target);
636657
}
637658
node.visitChildren(this);
638659
}
@@ -774,7 +795,7 @@ class CodeChecker extends RecursiveAstVisitor {
774795
op.type == TokenType.PLUS_PLUS ||
775796
op.type == TokenType.MINUS_MINUS) {
776797
if (_rules.isDynamicTarget(node.operand)) {
777-
_recordDynamicInvoke(node);
798+
_recordDynamicInvoke(node, node.operand);
778799
}
779800
// For ++ and --, even if it is not dynamic, we still need to check
780801
// that the user defined method accepts an `int` as the RHS.
@@ -790,7 +811,7 @@ class CodeChecker extends RecursiveAstVisitor {
790811
// Dynamic invocation
791812
// TODO(vsm): Move this logic to the resolver?
792813
if (op.type != TokenType.EQ_EQ && op.type != TokenType.BANG_EQ) {
793-
_recordDynamicInvoke(node);
814+
_recordDynamicInvoke(node, node.leftOperand);
794815
}
795816
} else {
796817
var element = node.staticElement;
@@ -831,8 +852,9 @@ class CodeChecker extends RecursiveAstVisitor {
831852

832853
@override
833854
void visitIndexExpression(IndexExpression node) {
834-
if (_rules.isDynamicTarget(node.target)) {
835-
_recordDynamicInvoke(node);
855+
var target = node.realTarget;
856+
if (_rules.isDynamicTarget(target)) {
857+
_recordDynamicInvoke(node, target);
836858
} else {
837859
var element = node.staticElement;
838860
if (element is MethodElement) {
@@ -897,7 +919,7 @@ class CodeChecker extends RecursiveAstVisitor {
897919
var methodElement = expr.staticElement;
898920
if (methodElement == null) {
899921
// Dynamic invocation
900-
_recordDynamicInvoke(expr);
922+
_recordDynamicInvoke(expr, expr.leftHandSide);
901923
} else {
902924
// Sanity check the operator
903925
assert(methodElement.isOperator);
@@ -941,8 +963,13 @@ class CodeChecker extends RecursiveAstVisitor {
941963
}
942964
}
943965

944-
void _recordDynamicInvoke(AstNode node) {
945-
_reporter.log(new DynamicInvoke(_rules, node));
966+
void _recordDynamicInvoke(AstNode node, AstNode target) {
967+
var dinvoke = new DynamicInvoke(_rules, node);
968+
_reporter.log(dinvoke);
969+
// TODO(jmesserly): we may eventually want to record if the whole operation
970+
// (node) was dynamic, rather than the target, but this is an easier fit
971+
// with what we used to do.
972+
DynamicInvoke.set(target, true);
946973
}
947974

948975
void _recordMessage(StaticInfo info) {

‎lib/src/codegen/js_codegen.dart

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,7 +1484,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
14841484
id = lhs.identifier;
14851485
}
14861486

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

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

1543-
if (rules.isDynamicTarget(target)) {
1543+
if (DynamicInvoke.get(target)) {
15441544
code = 'dart.$DSEND(#, #, #)';
1545-
} else if (rules.isDynamicCall(node.methodName)) {
1545+
} else if (DynamicInvoke.get(node.methodName)) {
15461546
// This is a dynamic call to a statically know target. For example:
15471547
// class Foo { Function bar; }
15481548
// new Foo().bar(); // dynamic call
@@ -1584,7 +1584,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
15841584
JS.Expression visitFunctionExpressionInvocation(
15851585
FunctionExpressionInvocation node) {
15861586
var code;
1587-
if (rules.isDynamicCall(node.function)) {
1587+
if (DynamicInvoke.get(node.function)) {
15881588
code = 'dart.$DCALL(#, #)';
15891589
} else {
15901590
code = '#(#)';
@@ -1995,6 +1995,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
19951995
new SimpleIdentifier(new StringToken(TokenType.IDENTIFIER, name, -1));
19961996
id.staticElement = new TemporaryVariableElement.forNode(id);
19971997
id.staticType = type;
1998+
DynamicInvoke.set(id, type.isDynamic);
19981999
return id;
19992000
}
20002001

@@ -2022,25 +2023,30 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
20222023
/// needed.
20232024
Expression _bindLeftHandSide(
20242025
Map<String, JS.Expression> scope, Expression expr, {Expression context}) {
2026+
Expression result;
20252027
if (expr is IndexExpression) {
20262028
IndexExpression index = expr;
2027-
return new IndexExpression.forTarget(
2029+
result = new IndexExpression.forTarget(
20282030
_bindValue(scope, 'o', index.target, context: context),
20292031
index.leftBracket,
20302032
_bindValue(scope, 'i', index.index, context: context),
2031-
index.rightBracket)..staticType = expr.staticType;
2033+
index.rightBracket);
20322034
} else if (expr is PropertyAccess) {
20332035
PropertyAccess prop = expr;
2034-
return new PropertyAccess(
2036+
result = new PropertyAccess(
20352037
_bindValue(scope, 'o', _getTarget(prop), context: context),
2036-
prop.operator, prop.propertyName)..staticType = expr.staticType;
2038+
prop.operator, prop.propertyName);
20372039
} else if (expr is PrefixedIdentifier) {
20382040
PrefixedIdentifier ident = expr;
2039-
return new PrefixedIdentifier(
2041+
result = new PrefixedIdentifier(
20402042
_bindValue(scope, 'o', ident.prefix, context: context), ident.period,
2041-
ident.identifier)..staticType = expr.staticType;
2043+
ident.identifier);
2044+
} else {
2045+
return expr as SimpleIdentifier;
20422046
}
2043-
return expr as SimpleIdentifier;
2047+
result.staticType = expr.staticType;
2048+
DynamicInvoke.set(result, DynamicInvoke.get(expr));
2049+
return result;
20442050
}
20452051

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

@@ -2246,7 +2252,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
22462252
Expression target, String name, List<Expression> args) {
22472253
var type = getStaticType(target);
22482254
var memberName = _emitMemberName(name, unary: args.isEmpty, type: type);
2249-
if (rules.isDynamicTarget(target)) {
2255+
if (DynamicInvoke.get(target)) {
22502256
// dynamic dispatch
22512257
var dynamicHelper = const {'[]': DINDEX, '[]=': DSETINDEX}[name];
22522258
if (dynamicHelper != null) {

‎lib/src/info.dart

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ abstract class CoercionInfo extends StaticInfo {
124124

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

127-
static const String _propertyName = 'dev_compiler.Conversion';
127+
static const String _propertyName = 'dev_compiler.src.info.CoercionInfo';
128128

129129
/// Gets the coercion info associated with this node.
130130
static CoercionInfo get(AstNode node) => node.getProperty(_propertyName);
@@ -345,6 +345,22 @@ class DynamicInvoke extends CoercionInfo {
345345
DartType get convertedType => rules.provider.dynamicType;
346346
String get message => '$node requires dynamic invoke';
347347
Level get level => Level.INFO;
348+
349+
static const String _propertyName = 'dev_compiler.src.info.DynamicInvoke';
350+
351+
/// Whether this [node] is the target of a dynamic operation.
352+
static bool get(AstNode node) {
353+
var value = node.getProperty(_propertyName);
354+
return value != null ? value : false;
355+
}
356+
357+
/// Sets whether this node is the target of a dynamic operation.
358+
static bool set(AstNode node, bool value) {
359+
// Free the storage for things that aren't dynamic.
360+
if (value == false) value = null;
361+
node.setProperty(_propertyName, value);
362+
return value;
363+
}
348364
}
349365

350366
abstract class StaticError extends StaticInfo {

0 commit comments

Comments
 (0)