Skip to content

Commit

Permalink
[fasta] Check type arguments in method invocations while doing inference
Browse files Browse the repository at this point in the history
Fixes #34899

Bug: http://dartbug.com/34899
Change-Id: I4db7612925d0938cc6ec381c4de8073d50d61aac
Reviewed-on: https://dart-review.googlesource.com/c/81266
Commit-Queue: Peter von der Ahé <[email protected]>
Reviewed-by: Peter von der Ahé <[email protected]>
Auto-Submit: Dmitry Stefantsov <[email protected]>
  • Loading branch information
Dmitry Stefantsov authored and [email protected] committed Oct 24, 2018
1 parent d693742 commit cf0a99a
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 60 deletions.
17 changes: 0 additions & 17 deletions pkg/front_end/lib/src/fasta/kernel/body_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,6 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
/// invocations are to be resolved in a separate step.
final List<Expression> redirectingFactoryInvocations = <Expression>[];

/// In some cases checks of the type arguments in method invocations can't be
/// done right away, because some type arguments within the receiver
/// expression are yet to be inferred.
final List<MethodInvocation> delayedBoundsChecks = <MethodInvocation>[];

BodyBuilder(
this.library,
this.member,
Expand Down Expand Up @@ -563,11 +558,6 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
}

resolveRedirectingFactoryTargets();
for (MethodInvocation node in delayedBoundsChecks) {
library.checkBoundsInMethodInvocation(
node, classBuilder?.target, typeEnvironment);
}
delayedBoundsChecks.clear();
}

@override
Expand Down Expand Up @@ -838,11 +828,6 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
}

resolveRedirectingFactoryTargets();
for (MethodInvocation node in delayedBoundsChecks) {
library.checkBoundsInMethodInvocation(
node, classBuilder?.target, typeEnvironment);
}
delayedBoundsChecks.clear();
}

void resolveRedirectingFactoryTargets() {
Expand Down Expand Up @@ -4571,7 +4556,6 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
receiver, callName, forest.castArguments(arguments),
isImplicitCall: true)
..fileOffset = forest.readOffset(arguments);
delayedBoundsChecks.add(node);
return node;
}

Expand All @@ -4594,7 +4578,6 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
receiver, name, forest.castArguments(arguments),
isImplicitCall: isImplicitCall, interfaceTarget: interfaceTarget)
..fileOffset = offset;
delayedBoundsChecks.add(node);
return node;
}
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -876,18 +876,10 @@ class InferenceVistor extends BodyVisitor1<void, DartType> {
}
}
}
bool hadExplicitTypeArguments =
getExplicitTypeArguments(node.arguments) != null;
var inferenceResult = inferrer.inferMethodInvocation(
node, node.receiver, node.fileOffset, node._isImplicitCall, typeContext,
desugaredInvocation: node);
node.inferredType = inferenceResult.type;
KernelLibraryBuilder inferrerLibrary = inferrer.library;
if (!hadExplicitTypeArguments && inferrerLibrary is KernelLibraryBuilder) {
inferrerLibrary.checkBoundsInMethodInvocation(
node, inferrer.thisType?.classNode, inferrer.typeSchemaEnvironment,
inferred: true);
}
}

void visitNamedFunctionExpressionJudgment(
Expand Down
40 changes: 16 additions & 24 deletions pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import 'package:kernel/ast.dart'
ListLiteral,
MapLiteral,
Member,
MethodInvocation,
Name,
Procedure,
ProcedureKind,
Expand Down Expand Up @@ -94,6 +93,8 @@ import '../source/source_class_builder.dart' show SourceClassBuilder;
import '../source/source_library_builder.dart'
show DeclarationBuilder, SourceLibraryBuilder;

import '../type_inference/type_inferrer.dart' show TypeInferrerImpl;

import 'kernel_builder.dart'
show
AccessErrorBuilder,
Expand Down Expand Up @@ -1814,24 +1815,16 @@ class KernelLibraryBuilder
}

void checkBoundsInMethodInvocation(
MethodInvocation node, Class thisClass, TypeEnvironment typeEnvironment,
DartType receiverType,
TypeEnvironment typeEnvironment,
TypeInferrerImpl typeInferrer,
Name name,
Member interfaceTarget,
Arguments arguments,
int offset,
{bool inferred = false}) {
if (!loader.target.strongMode) return;
if (node.arguments.types.isEmpty) return;
DartType savedThisType = typeEnvironment.thisType;
if (thisClass != null) {
typeEnvironment.thisType = new InterfaceType(
thisClass,
thisClass.typeParameters
.map((p) => new TypeParameterType(p))
.toList());
}
DartType receiverType;
try {
receiverType = node.receiver.getStaticType(typeEnvironment);
} finally {
typeEnvironment.thisType = savedThisType;
}
if (arguments.types.isEmpty) return;
Class klass;
List<DartType> klassArguments;
if (receiverType is InterfaceType) {
Expand All @@ -1845,15 +1838,14 @@ class KernelLibraryBuilder
substitutionMap[klass.typeParameters[i]] = klassArguments[i];
}
// TODO(dmitryas): Find a better way than relying on [interfaceTarget].
Member method =
typeEnvironment.hierarchy.getDispatchTarget(klass, node.name) ??
node.interfaceTarget;
Member method = typeEnvironment.hierarchy.getDispatchTarget(klass, name) ??
interfaceTarget;
if (method == null || method is! Procedure) {
return;
}
List<TypeParameter> methodParameters = method.function.typeParameters;
// The error is to be reported elsewhere.
if (methodParameters.length != node.arguments.types.length) return;
if (methodParameters.length != arguments.types.length) return;
List<TypeParameter> instantiatedMethodParameters =
new List<TypeParameter>.filled(methodParameters.length, null);
for (int i = 0; i < instantiatedMethodParameters.length; ++i) {
Expand All @@ -1867,7 +1859,7 @@ class KernelLibraryBuilder
substitute(methodParameters[i].bound, substitutionMap);
}
List<Object> violations = typeEnvironment.findBoundViolationsElementwise(
instantiatedMethodParameters, node.arguments.types,
instantiatedMethodParameters, arguments.types,
typedefInstantiations: typedefInstantiations);
if (violations != null) {
String targetName = "${klass.name}";
Expand All @@ -1878,7 +1870,7 @@ class KernelLibraryBuilder
}
targetName += ">";
}
targetName += "::${node.name.name}";
targetName += "::${name.name}";
for (int i = 0; i < violations.length; i += 3) {
DartType argument = violations[i];
TypeParameter variable = violations[i + 1];
Expand Down Expand Up @@ -1906,7 +1898,7 @@ class KernelLibraryBuilder
}
}

reportBoundViolation(message, node.fileOffset, variable);
reportBoundViolation(message, offset, variable);
}
}
}
Expand Down
34 changes: 34 additions & 0 deletions pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,7 @@ abstract class TypeInferrerImpl extends TypeInferrer {
interfaceMember, receiverType);
var calleeType = getCalleeType(interfaceMember, receiverType);
var functionType = getCalleeFunctionType(calleeType, !isImplicitCall);

if (interfaceMember != null &&
calleeType is! DynamicType &&
calleeType != coreTypes.functionClass.rawType &&
Expand Down Expand Up @@ -1641,6 +1642,39 @@ abstract class TypeInferrerImpl extends TypeInferrer {
parent?.replaceChild(expression, errorNode);
}
}

// If [arguments] were inferred, check them.
// TODO(dmitryas): Figure out why [library] is sometimes null.
if (library != null) {
// [actualReceiverType], [interfaceTarget], and [actualMethodName] below
// are for a workaround for the cases like the following:
//
// class C1 { var f = new C2(); }
// class C2 { int call<X extends num>(X x) => 42; }
// main() { C1 c = new C1(); c.f("foobar"); }
DartType actualReceiverType;
Member interfaceTarget;
Name actualMethodName;
if (calleeType is InterfaceType) {
actualReceiverType = calleeType;
interfaceTarget = null;
actualMethodName = callName;
} else {
actualReceiverType = receiverType;
interfaceTarget = interfaceMember is Member ? interfaceMember : null;
actualMethodName = methodName;
}
library.checkBoundsInMethodInvocation(
actualReceiverType,
typeSchemaEnvironment,
this,
actualMethodName,
interfaceTarget,
arguments,
fileOffset,
inferred: getExplicitTypeArguments(arguments) == null);
}

return new ExpressionInferenceResult(null, inferredType);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,13 @@
// Try specifying type arguments explicitly so that they conform to the bounds.
// . /*error:COULD_NOT_INFER*/ /*@typeArgs=int*/ /*@target=Foo::method*/ method(
// ^
//
// pkg/front_end/testcases/inference/generic_methods_correctly_recognize_generic_upper_bound.dart:26:77: Error: Type argument 'dart.core::int' violates the corresponding type variable bound of 'Foo<dart.core::String>::method'.
// Try changing type arguments so that they conform to the bounds.
// . /*error:COULD_NOT_INFER*/ /*@typeArgs=int*/ /*@target=Foo::method*/ method(
// ^

// Unhandled errors:
//
// pkg/front_end/testcases/inference/generic_methods_correctly_recognize_generic_upper_bound.dart:26:77: Error: Inferred type argument 'dart.core::int' violates the corresponding type variable bound of 'Foo<dart.core::String>::method'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// . /*error:COULD_NOT_INFER*/ /*@typeArgs=int*/ /*@target=Foo::method*/ method(
// ^
//
// pkg/front_end/testcases/inference/generic_methods_correctly_recognize_generic_upper_bound.dart:26:77: Error: Type argument 'dart.core::int' violates the corresponding type variable bound of 'Foo<dart.core::String>::method'.
// Try changing type arguments so that they conform to the bounds.
// . /*error:COULD_NOT_INFER*/ /*@typeArgs=int*/ /*@target=Foo::method*/ method(
// ^

library test;
import self as self;
Expand Down
36 changes: 36 additions & 0 deletions pkg/front_end/testcases/issue34899.dart.strong.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
library;
import self as self;
import "dart:core" as core;
import "dart:async" as asy;

class Foo<T extends core::Object = dynamic> extends core::Object {
final field () → asy::Future<dynamic> quux;
generic-covariant-impl field self::Foo::T t;
constructor •(() → asy::Future<dynamic> quux, self::Foo::T t) → self::Foo<self::Foo::T>
: self::Foo::quux = quux, self::Foo::t = t, super core::Object::•()
;
method call() → asy::Future<self::Foo::T>
return this.{self::Foo::quux}().{asy::Future::then}<self::Foo::T>((dynamic _) → self::Foo::T => this.{self::Foo::t});
}
class Bar extends core::Object {
field self::Foo<self::Baz> qux = null;
synthetic constructor •() → self::Bar
: super core::Object::•()
;
method quuz() → asy::Future<void>
return this.{self::Bar::qux}().{asy::Future::then}<self::Grault>((self::Baz baz) → self::Grault => this.{self::Bar::corge}(baz)).{asy::Future::then}<void>((self::Grault grault) → void => this.{self::Bar::garply}(grault));
method corge(self::Baz baz) → self::Grault
return null;
method garply(self::Grault grault) → void {}
}
class Baz extends core::Object {
synthetic constructor •() → self::Baz
: super core::Object::•()
;
}
class Grault extends core::Object {
synthetic constructor •() → self::Grault
: super core::Object::•()
;
}
static method main() → dynamic {}
2 changes: 1 addition & 1 deletion pkg/front_end/testcases/strong.status
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ instantiate_to_bound/body_typedef_super_bounded_type: Fail # Issue 33444
instantiate_to_bound/typedef_super_bounded_type: Fail # Issue 33444
invalid_type: TypeCheckError
invocations: Fail
issue34899: Crash
issue34899: TypeCheckError
literals: Fail
map: Fail
micro: Fail
Expand Down

0 comments on commit cf0a99a

Please sign in to comment.