Store literal values and invocations arguments only for constants and untyped literals.

1. When a literal is typed, and is not a constant, we don't need its
   elements for type inference. Moreover, in this case elements are
   allowed to include nodes that are not immediately-evident expressions,
   such as instance creations without all type arguments, closures with
   block bodies, etc.

2. When a literal is a constant, so we still have to serialize all its
   elements, it is possible that we will encounter something not allowed
   for immediately-evident expressions, and fail. But that's OK, because
   such expressions are also not allowed in constants, so we will have
   a compile time error anyway, and IMHO it is OK to degrade a little
   in presence of errors.

3. The same is true for method invocations and instance creations,
   we need to serialize arguments only for constants, because top-level
   inference never uses arguments to infer returned types.

R=brianwilkerson@google.com
BUG=

Review-Url: https://codereview.chromium.org/2779993002 .
This commit is contained in:
Konstantin Shcheglov 2017-03-28 14:13:52 -07:00
parent a5ebc58cef
commit 84a4b01081
7 changed files with 171 additions and 159 deletions

View file

@ -525,6 +525,11 @@ enum UnlinkedExprOperation : byte {
* from [UnlinkedExpr.references], and push the resulting value back onto the
* stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* Note that for an invocation of the form `const a.b(...)` (where no type
* arguments are specified), it is impossible to tell from the unresolved AST
* alone whether `a` is a class name and `b` is a constructor name, or `a` is
@ -768,6 +773,11 @@ enum UnlinkedExprOperation : byte {
* aforementioned method or function. Push the result of the invocation onto
* the stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* In general `a.b` cannot not be distinguished between: `a` is a prefix and
* `b` is a top-level function; or `a` is an object and `b` is the name of a
* method. This operation should be used for a sequence of identifiers
@ -789,6 +799,11 @@ enum UnlinkedExprOperation : byte {
* arguments for the aforementioned method. Push the result of the
* invocation onto the stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* This operation should be used for invocation of a method invocation
* where `target` is known to be an object instance.
*/

View file

@ -2197,6 +2197,11 @@ enum UnlinkedExprOperation {
* from [UnlinkedExpr.references], and push the resulting value back onto the
* stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* Note that for an invocation of the form `const a.b(...)` (where no type
* arguments are specified), it is impossible to tell from the unresolved AST
* alone whether `a` is a class name and `b` is a constructor name, or `a` is
@ -2440,6 +2445,11 @@ enum UnlinkedExprOperation {
* aforementioned method or function. Push the result of the invocation onto
* the stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* In general `a.b` cannot not be distinguished between: `a` is a prefix and
* `b` is a top-level function; or `a` is an object and `b` is the name of a
* method. This operation should be used for a sequence of identifiers
@ -2461,6 +2471,11 @@ enum UnlinkedExprOperation {
* arguments for the aforementioned method. Push the result of the
* invocation onto the stack.
*
* Arguments are skipped, and `0` are specified as the numbers of arguments
* on the stack, if the expression is not a constant. We store expression of
* variable initializers to perform top-level inference, and arguments are
* never used to infer types.
*
* This operation should be used for invocation of a method invocation
* where `target` is known to be an object instance.
*/

View file

@ -43,8 +43,9 @@ class _ConstExprSerializer extends AbstractConstExprSerializer {
*/
final Set<String> parameterNames;
_ConstExprSerializer(
this.visitor, this.localClosureIndexMap, this.parameterNames);
_ConstExprSerializer(bool forConst, this.visitor, this.localClosureIndexMap,
this.parameterNames)
: super(forConst);
@override
bool isParameterName(String name) {
@ -413,7 +414,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
// localClosureIndexMap.
Map<int, int> localClosureIndexMap = null;
_ConstExprSerializer serializer =
new _ConstExprSerializer(this, localClosureIndexMap, null);
new _ConstExprSerializer(true, this, localClosureIndexMap, null);
try {
serializer.serializeAnnotation(a);
} on StateError {
@ -549,10 +550,10 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
* Serialize the given [expression], creating an [UnlinkedExprBuilder].
*/
UnlinkedExprBuilder serializeConstExpr(
Map<int, int> localClosureIndexMap, Expression expression,
bool forConst, Map<int, int> localClosureIndexMap, Expression expression,
[Set<String> parameterNames]) {
_ConstExprSerializer serializer =
new _ConstExprSerializer(this, localClosureIndexMap, parameterNames);
_ConstExprSerializer serializer = new _ConstExprSerializer(
forConst, this, localClosureIndexMap, parameterNames);
serializer.serialize(expression);
return serializer.toBuilder();
}
@ -692,7 +693,8 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
_parameterNames.addAll(formalParameters.parameters
.map((FormalParameter p) => p.identifier.name));
}
serializeFunctionBody(b, null, body, serializeBodyExpr, serializeBody);
serializeFunctionBody(
b, null, body, serializeBodyExpr, serializeBody, false);
_parameterNames = oldParameterNames;
scopes.removeLast();
assert(scopes.length == oldScopesLength);
@ -720,7 +722,8 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
List<ConstructorInitializer> initializers,
AstNode body,
bool serializeBodyExpr,
bool serializeBody) {
bool serializeBody,
bool forConst) {
if (body is BlockFunctionBody || body is ExpressionFunctionBody) {
for (UnlinkedParamBuilder parameter in b.parameters) {
if (!parameter.isInitializingFormal) {
@ -749,11 +752,11 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
}
if (serializeBodyExpr) {
if (body is Expression) {
b.bodyExpr =
serializeConstExpr(_localClosureIndexMap, body, _parameterNames);
b.bodyExpr = serializeConstExpr(
forConst, _localClosureIndexMap, body, _parameterNames);
} else if (body is ExpressionFunctionBody) {
b.bodyExpr = serializeConstExpr(
_localClosureIndexMap, body.expression, _parameterNames);
forConst, _localClosureIndexMap, body.expression, _parameterNames);
} else {
// TODO(paulberry): serialize other types of function bodies.
}
@ -816,14 +819,14 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
* in [UnlinkedExecutableBuilder.bodyExpr].
*/
UnlinkedExecutableBuilder serializeInitializerFunction(
Expression expression, bool serializeBodyExpr) {
Expression expression, bool serializeBodyExpr, bool forConst) {
if (expression == null) {
return null;
}
UnlinkedExecutableBuilder initializer =
new UnlinkedExecutableBuilder(nameOffset: expression.offset);
serializeFunctionBody(
initializer, null, expression, serializeBodyExpr, true);
initializer, null, expression, serializeBodyExpr, true, forConst);
initializer.inferredReturnTypeSlot = assignSlot();
return initializer;
}
@ -1023,8 +1026,8 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
bool serializeBodyExpr = variable.isConst ||
variable.isFinal && isField && !isDeclaredStatic ||
variables.type == null;
b.initializer =
serializeInitializerFunction(variable.initializer, serializeBodyExpr);
b.initializer = serializeInitializerFunction(
variable.initializer, serializeBodyExpr, b.isConst);
if (isField && !isDeclaredStatic && !variables.isFinal) {
b.inheritsCovariantSlot = assignSlot();
}
@ -1126,7 +1129,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
// don't need a localClosureIndexMap.
Map<int, int> localClosureIndexMap = null;
b.redirectedConstructor =
new _ConstExprSerializer(this, localClosureIndexMap, null)
new _ConstExprSerializer(true, this, localClosureIndexMap, null)
.serializeConstructorRef(null, typeName.name,
typeName.typeArguments, node.redirectedConstructor.name);
}
@ -1147,16 +1150,16 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
b.documentationComment = serializeDocumentation(node.documentationComment);
b.annotations = serializeAnnotations(node.metadata);
b.codeRange = serializeCodeRange(node);
Map<int, int> localClosureIndexMap = serializeFunctionBody(
b, node.initializers, node.body, node.constKeyword != null, false);
Map<int, int> localClosureIndexMap = serializeFunctionBody(b,
node.initializers, node.body, node.constKeyword != null, false, false);
if (node.constKeyword != null) {
Set<String> constructorParameterNames =
node.parameters.parameters.map((p) => p.identifier.name).toSet();
b.constantInitializers = node.initializers
.map((ConstructorInitializer initializer) =>
serializeConstructorInitializer(initializer, (Expression expr) {
return serializeConstExpr(
localClosureIndexMap, expr, constructorParameterNames);
return serializeConstExpr(true, localClosureIndexMap, expr,
constructorParameterNames);
}))
.toList();
}
@ -1168,7 +1171,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
DefaultFormalParameter node) {
UnlinkedParamBuilder b =
node.parameter.accept(this) as UnlinkedParamBuilder;
b.initializer = serializeInitializerFunction(node.defaultValue, true);
b.initializer = serializeInitializerFunction(node.defaultValue, true, true);
if (node.defaultValue != null) {
b.defaultValueCode = node.defaultValue.toSource();
}

View file

@ -68,6 +68,16 @@ UnlinkedConstructorInitializer serializeConstructorInitializer(
* serialization of a single constant [Expression].
*/
abstract class AbstractConstExprSerializer {
/**
* Whether an expression that should be a constant is being serialized.
*
* For constants we need to store more than we need just for type inference,
* because we need to be able to restore these AST to evaluate actual values
* of constants. So, we need to store constructor arguments, elements for
* list and map literals even if these literals are typed.
*/
final bool forConst;
/**
* See [UnlinkedExprBuilder.isValidConst].
*/
@ -109,6 +119,8 @@ abstract class AbstractConstExprSerializer {
*/
final List<EntityRefBuilder> references = <EntityRefBuilder>[];
AbstractConstExprSerializer(this.forConst);
/**
* Return `true` if the given [name] is a parameter reference.
*/
@ -413,21 +425,26 @@ abstract class AbstractConstExprSerializer {
}
void _serializeArguments(ArgumentList argumentList) {
List<Expression> arguments = argumentList.arguments;
// Serialize the arguments.
List<String> argumentNames = <String>[];
arguments.forEach((arg) {
if (arg is NamedExpression) {
argumentNames.add(arg.name.label.name);
_serialize(arg.expression);
} else {
_serialize(arg);
}
});
// Add numbers of named and positional arguments, and the op-code.
ints.add(argumentNames.length);
strings.addAll(argumentNames);
ints.add(arguments.length - argumentNames.length);
if (forConst) {
List<Expression> arguments = argumentList.arguments;
// Serialize the arguments.
List<String> argumentNames = <String>[];
arguments.forEach((arg) {
if (arg is NamedExpression) {
argumentNames.add(arg.name.label.name);
_serialize(arg.expression);
} else {
_serialize(arg);
}
});
// Add numbers of named and positional arguments, and the op-code.
ints.add(argumentNames.length);
strings.addAll(argumentNames);
ints.add(arguments.length - argumentNames.length);
} else {
ints.add(0);
ints.add(0);
}
}
void _serializeAssignment(AssignmentExpression expr) {
@ -521,9 +538,13 @@ abstract class AbstractConstExprSerializer {
}
void _serializeListLiteral(ListLiteral expr) {
List<Expression> elements = expr.elements;
elements.forEach(_serialize);
ints.add(elements.length);
if (forConst || expr.typeArguments == null) {
List<Expression> elements = expr.elements;
elements.forEach(_serialize);
ints.add(elements.length);
} else {
ints.add(0);
}
if (expr.typeArguments != null &&
expr.typeArguments.arguments.length == 1) {
references.add(serializeTypeName(expr.typeArguments.arguments[0]));
@ -534,11 +555,15 @@ abstract class AbstractConstExprSerializer {
}
void _serializeMapLiteral(MapLiteral expr) {
for (MapLiteralEntry entry in expr.entries) {
_serialize(entry.key);
_serialize(entry.value);
if (forConst || expr.typeArguments == null) {
for (MapLiteralEntry entry in expr.entries) {
_serialize(entry.key);
_serialize(entry.value);
}
ints.add(expr.entries.length);
} else {
ints.add(0);
}
ints.add(expr.entries.length);
if (expr.typeArguments != null &&
expr.typeArguments.arguments.length == 2) {
references.add(serializeTypeName(expr.typeArguments.arguments[0]));

View file

@ -516,27 +516,6 @@ class D {
expect(classD.fields[0].inferredType.toString(), 'dynamic');
}
void test_inferredTypeFromOutsideBuildUnit_methodParamType_viaGeneric() {
var bundle = createPackageBundle(
'''
class B {
T f<T>(T t) => t;
}
class C extends B {
f<T>(t) => t; // Inferred param type: T
}
''',
path: '/a.dart');
addBundle('/a.ds', bundle);
createLinker('''
import 'a.dart';
var x = new C().f(0); // Inferred type: int
''');
LibraryElementForLink library = linker.getLibrary(linkerInputs.testDartUri);
expect(_getVariable(library.getContainedName('x')).inferredType.toString(),
'int');
}
void test_inferredTypeFromOutsideBuildUnit_methodParamType_viaInheritance() {
var bundle = createPackageBundle(
'''

View file

@ -7099,9 +7099,6 @@ final v = new C().f;
}
test_expr_functionExpression_asArgument() {
if (skipNonConstInitializers) {
return;
}
UnlinkedVariable variable = serializeVariableText('''
final v = foo(5, () => 42);
foo(a, b) {}
@ -7109,16 +7106,11 @@ foo(a, b) {}
assertUnlinkedConst(variable.initializer.bodyExpr,
isValidConst: false,
operators: [
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushLocalFunctionReference,
UnlinkedExprOperation.invokeMethodRef
],
ints: [
5,
0,
0,
0,
2,
0
],
referenceValidators: [
@ -7128,9 +7120,6 @@ foo(a, b) {}
}
test_expr_functionExpression_asArgument_multiple() {
if (skipNonConstInitializers) {
return;
}
UnlinkedVariable variable = serializeVariableText('''
final v = foo(5, () => 42, () => 43);
foo(a, b, c) {}
@ -7138,19 +7127,11 @@ foo(a, b, c) {}
assertUnlinkedConst(variable.initializer.bodyExpr,
isValidConst: false,
operators: [
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushLocalFunctionReference,
UnlinkedExprOperation.pushLocalFunctionReference,
UnlinkedExprOperation.invokeMethodRef
],
ints: [
5,
0,
0,
0,
1,
0,
3,
0
],
referenceValidators: [
@ -7412,9 +7393,6 @@ class C<T> {
}
test_expr_invokeMethod_instance() {
if (skipNonConstInitializers) {
return;
}
UnlinkedVariable variable = serializeVariableText('''
class C {
int m(a, {b, c}) => 42;
@ -7425,24 +7403,16 @@ final v = new C().m(1, b: 2, c: 3);
isValidConst: false,
operators: [
UnlinkedExprOperation.invokeConstructor,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.invokeMethod,
],
ints: [
0,
0,
1,
2,
3,
2,
1,
0,
0,
0
],
strings: [
'b',
'c',
'm'
],
referenceValidators: [
@ -7504,15 +7474,11 @@ final v = a.b.c.m(10, 20);
assertUnlinkedConst(variable.initializer.bodyExpr,
isValidConst: false,
operators: [
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.invokeMethodRef,
],
ints: [
10,
20,
0,
2,
0,
0
],
strings: [],
@ -7577,17 +7543,14 @@ final v = f(u);
assertUnlinkedConst(variable.initializer.bodyExpr,
isValidConst: false,
operators: [
UnlinkedExprOperation.pushReference,
UnlinkedExprOperation.invokeMethodRef
],
ints: [
0,
1,
0,
0
],
referenceValidators: [
(EntityRef r) => checkTypeRef(r, null, 'u',
expectedKind: ReferenceKind.topLevelPropertyAccessor),
(EntityRef r) => checkTypeRef(r, null, 'f',
expectedKind: ReferenceKind.topLevelFunction)
]);
@ -7620,6 +7583,72 @@ final v = f<int, String>();
]);
}
test_expr_makeTypedList() {
UnlinkedVariable variable =
serializeVariableText('var v = <int>[11, 22, 33];');
assertUnlinkedConst(variable.initializer.bodyExpr, operators: [
UnlinkedExprOperation.makeTypedList
], ints: [
0
], referenceValidators: [
(EntityRef r) => checkTypeRef(r, 'dart:core', 'int',
expectedKind: ReferenceKind.classOrEnum)
]);
}
test_expr_makeTypedMap() {
UnlinkedVariable variable = serializeVariableText(
'var v = <int, String>{11: "aaa", 22: "bbb", 33: "ccc"};');
assertUnlinkedConst(variable.initializer.bodyExpr, operators: [
UnlinkedExprOperation.makeTypedMap
], ints: [
0
], referenceValidators: [
(EntityRef r) => checkTypeRef(r, 'dart:core', 'int',
expectedKind: ReferenceKind.classOrEnum),
(EntityRef r) => checkTypeRef(r, 'dart:core', 'String',
expectedKind: ReferenceKind.classOrEnum)
]);
}
test_expr_makeUntypedList() {
UnlinkedVariable variable = serializeVariableText('var v = [11, 22, 33];');
assertUnlinkedConst(variable.initializer.bodyExpr, operators: [
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.makeUntypedList
], ints: [
11,
22,
33,
3
]);
}
test_expr_makeUntypedMap() {
UnlinkedVariable variable =
serializeVariableText('var v = {11: "aaa", 22: "bbb", 33: "ccc"};');
assertUnlinkedConst(variable.initializer.bodyExpr, operators: [
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushString,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushString,
UnlinkedExprOperation.pushInt,
UnlinkedExprOperation.pushString,
UnlinkedExprOperation.makeUntypedMap
], ints: [
11,
22,
33,
3
], strings: [
'aaa',
'bbb',
'ccc'
]);
}
test_expr_super() {
if (skipNonConstInitializers) {
return;

View file

@ -4442,42 +4442,6 @@ class C {
expect(v.type.toString(), '(String) → int');
}
test_typeInferenceDependency_staticVariable_inIdentifierSequence() async {
// Check that type inference dependencies are properly checked when a static
// variable appears in the middle of a string of identifiers separated by
// '.'.
var mainUnit = await checkFileElement('''
final a = /*info:DYNAMIC_INVOKE*/C.d.i;
class C {
static final d = new D(a);
}
class D {
D(_);
int i;
}
''');
// No type should be inferred for a because there is a circular reference
// between a and C.d.
var a = mainUnit.topLevelVariables[0];
expect(a.type.toString(), 'dynamic');
}
test_typeInferenceDependency_topLevelVariable_inIdentifierSequence() async {
// Check that type inference dependencies are properly checked when a top
// level variable appears at the beginning of a string of identifiers
// separated by '.'.
await checkFileElement('''
final a = /*info:DYNAMIC_INVOKE*/c.i;
final c = new C(a);
class C {
C(_);
int i;
}
''');
// No type should be inferred for a because there is a circular reference
// between a and c.
}
test_unsafeBlockClosureInference_closureCall() async {
// Regression test for https://github.com/dart-lang/sdk/issues/26962
var mainUnit = await checkFileElement('''
@ -4997,18 +4961,6 @@ class InferredTypeTest_Driver extends InferredTypeTest {
await super.test_circularReference_viaClosures_initializerTypes();
}
@failingTest
@override
test_genericMethods_usesGreatestLowerBound_comment_topLevel() async {
await super.test_genericMethods_usesGreatestLowerBound_comment_topLevel();
}
@failingTest
@override
test_inferredType_customIndexOp() async {
await super.test_voidReturnTypeSubtypesDynamic();
}
@failingTest
@override
test_listLiteralsCanInferNull_topLevel() =>
@ -5046,10 +4998,4 @@ class InferredTypeTest_Driver extends InferredTypeTest {
await super
.test_unsafeBlockClosureInference_functionCall_explicitTypeParam_viaExpr2_comment();
}
@failingTest
@override
test_voidReturnTypeSubtypesDynamic() async {
await super.test_voidReturnTypeSubtypesDynamic();
}
}