Consider initializing formals during top-level type inference
The previous implementation assumed that initializing formals without
type annotations could be inferred after the rest of top-level type
inference was completely done. This is not necessarily the case
because inferred types for initializing formals depend on inferred
types for fields which in turn depend on inferred types for
constructors used in their initializers.
Fixes https://github.com/dart-lang/sdk/issues/32866
Change-Id: I9de0b865c740d7542e5f5ad3d62c4c47c4532266
Reviewed-on: https://dart-review.googlesource.com/60140
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_formal_parameter_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_formal_parameter_builder.dart
index 1830ea2..752a461 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_formal_parameter_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_formal_parameter_builder.dart
@@ -46,10 +46,6 @@
isFieldFormal: hasThis,
isCovariant: isCovariant)
..fileOffset = charOffset;
- if (type == null && hasThis) {
- library.loader.typeInferenceEngine
- .recordInitializingFormal(declaration);
- }
}
return declaration;
}
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart
index 935acb9..c78baa0 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart
@@ -444,6 +444,15 @@
return isRedirectingGenerativeConstructorImplementation(constructor);
}
+ bool get isEligibleForTopLevelInference {
+ if (formals != null) {
+ for (var formal in formals) {
+ if (formal.type == null && formal.hasThis) return true;
+ }
+ }
+ return false;
+ }
+
Constructor build(SourceLibraryBuilder library) {
if (constructor.name == null) {
constructor.function = buildFunction(library);
@@ -455,6 +464,14 @@
constructor.isExternal = isExternal;
constructor.name = new Name(name, library.target);
}
+ if (!library.disableTypeInference && isEligibleForTopLevelInference) {
+ for (KernelFormalParameterBuilder formal in formals) {
+ if (formal.type == null && formal.hasThis) {
+ formal.declaration.type = null;
+ }
+ }
+ library.loader.typeInferenceEngine.toBeInferred[constructor] = library;
+ }
return constructor;
}
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart
index f72a8a2..36ec248 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart
@@ -34,7 +34,10 @@
InstrumentationValueForTypeArgs;
import '../fasta_codes.dart'
- show noLength, templateCantUseSuperBoundedTypeForInstanceCreation;
+ show
+ noLength,
+ templateCantInferTypeDueToCircularity,
+ templateCantUseSuperBoundedTypeForInstanceCreation;
import '../problems.dart' show unhandled, unsupported;
@@ -607,6 +610,36 @@
@override
DartType _inferExpression(ShadowTypeInferrer inferrer, DartType typeContext) {
+ var library = inferrer.engine.beingInferred[target];
+ if (library != null) {
+ // There is a cyclic dependency where inferring the types of the
+ // initializing formals of a constructor required us to infer the
+ // corresponding field type which required us to know the type of the
+ // constructor.
+ var name = target.enclosingClass.name;
+ if (target.name.name != '') name += '.${target.name.name}';
+ library.addProblem(
+ templateCantInferTypeDueToCircularity.withArguments(name),
+ target.fileOffset,
+ name.length,
+ target.fileUri);
+ for (var declaration in target.function.positionalParameters) {
+ declaration.type ??= const DynamicType();
+ }
+ for (var declaration in target.function.namedParameters) {
+ declaration.type ??= const DynamicType();
+ }
+ } else if ((library = inferrer.engine.toBeInferred[target]) != null) {
+ inferrer.engine.toBeInferred.remove(target);
+ inferrer.engine.beingInferred[target] = library;
+ for (var declaration in target.function.positionalParameters) {
+ inferrer.engine.inferInitializingFormal(declaration, target);
+ }
+ for (var declaration in target.function.namedParameters) {
+ inferrer.engine.inferInitializingFormal(declaration, target);
+ }
+ inferrer.engine.beingInferred.remove(target);
+ }
var inferredType = inferrer.inferInvocation(
typeContext,
fileOffset,
@@ -807,7 +840,7 @@
/// Concrete shadow object representing a field in kernel form.
class ShadowField extends Field implements ShadowMember {
@override
- InferenceNode _inferenceNode;
+ InferenceNode inferenceNode;
ShadowTypeInferrer _typeInferrer;
@@ -823,13 +856,13 @@
}
static bool hasTypeInferredFromInitializer(ShadowField field) =>
- field._inferenceNode is FieldInitializerInferenceNode;
+ field.inferenceNode is FieldInitializerInferenceNode;
static bool isImplicitlyTyped(ShadowField field) => field._isImplicitlyTyped;
static void setInferenceNode(ShadowField field, InferenceNode node) {
- assert(field._inferenceNode == null);
- field._inferenceNode = node;
+ assert(field.inferenceNode == null);
+ field.inferenceNode = node;
}
}
@@ -1428,18 +1461,18 @@
abstract class ShadowMember implements Member {
Uri get fileUri;
- InferenceNode get _inferenceNode;
+ InferenceNode get inferenceNode;
- void set _inferenceNode(InferenceNode value);
+ void set inferenceNode(InferenceNode value);
void setInferredType(
TypeInferenceEngine engine, Uri uri, DartType inferredType);
static void resolveInferenceNode(Member member) {
if (member is ShadowMember) {
- if (member._inferenceNode != null) {
- member._inferenceNode.resolve();
- member._inferenceNode = null;
+ if (member.inferenceNode != null) {
+ member.inferenceNode.resolve();
+ member.inferenceNode = null;
}
}
}
@@ -1568,7 +1601,7 @@
/// Concrete shadow object representing a procedure in kernel form.
class ShadowProcedure extends Procedure implements ShadowMember {
@override
- InferenceNode _inferenceNode;
+ InferenceNode inferenceNode;
final bool _hasImplicitReturnType;
@@ -1757,9 +1790,9 @@
if (write is StaticSet) {
writeContext = write.target.setterType;
writeMember = write.target;
- if (writeMember is ShadowField && writeMember._inferenceNode != null) {
- writeMember._inferenceNode.resolve();
- writeMember._inferenceNode = null;
+ if (writeMember is ShadowField && writeMember.inferenceNode != null) {
+ writeMember.inferenceNode.resolve();
+ writeMember.inferenceNode = null;
}
}
var inferredResult = _inferRhs(inferrer, readType, writeContext);
@@ -1776,9 +1809,9 @@
@override
DartType _inferExpression(ShadowTypeInferrer inferrer, DartType typeContext) {
var target = this.target;
- if (target is ShadowField && target._inferenceNode != null) {
- target._inferenceNode.resolve();
- target._inferenceNode = null;
+ if (target is ShadowField && target.inferenceNode != null) {
+ target.inferenceNode.resolve();
+ target.inferenceNode = null;
}
var type = target.getterType;
if (target is Procedure && target.kind == ProcedureKind.Method) {
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
index 5a912e3..c436163 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
@@ -514,10 +514,12 @@
Constructor constructor, Map<TypeParameter, DartType> substitutionMap) {
VariableDeclaration copyFormal(VariableDeclaration formal) {
// TODO(ahe): Handle initializers.
- return new VariableDeclaration(formal.name,
- type: substitute(formal.type, substitutionMap),
- isFinal: formal.isFinal,
- isConst: formal.isConst);
+ var copy = new VariableDeclaration(formal.name,
+ isFinal: formal.isFinal, isConst: formal.isConst);
+ if (formal.type != null) {
+ copy.type = substitute(formal.type, substitutionMap);
+ }
+ return copy;
}
List<VariableDeclaration> positionalParameters = <VariableDeclaration>[];
diff --git a/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart b/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
index 5a52a48..9fe590f 100644
--- a/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
+++ b/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
@@ -4,16 +4,16 @@
import 'package:kernel/ast.dart'
show
- Class,
+ Constructor,
DartType,
DartTypeVisitor,
DynamicType,
FunctionType,
InterfaceType,
- Location,
TypeParameter,
TypeParameterType,
- TypedefType;
+ TypedefType,
+ VariableDeclaration;
import 'package:kernel/class_hierarchy.dart';
import 'package:kernel/core_types.dart';
@@ -21,12 +21,9 @@
import '../builder/library_builder.dart';
-import '../deprecated_problems.dart' show Crash;
-
import '../kernel/kernel_shadow_ast.dart';
-import '../messages.dart'
- show getLocationFromNode, noLength, templateCantInferTypeDueToCircularity;
+import '../messages.dart' show noLength, templateCantInferTypeDueToCircularity;
import '../source/source_library_builder.dart';
@@ -211,7 +208,20 @@
final staticInferenceNodes = <FieldInitializerInferenceNode>[];
- final initializingFormals = <ShadowVariableDeclaration>[];
+ /// A map containing constructors with initializing formals whose types
+ /// need to be inferred.
+ ///
+ /// This is represented as a map from a constructor to its library
+ /// builder because the builder is used to report errors due to cyclic
+ /// inference dependencies.
+ final Map<Constructor, LibraryBuilder> toBeInferred = {};
+
+ /// A map containing constructors in the process of being inferred.
+ ///
+ /// This is used to detect cyclic inference dependencies. It is represented
+ /// as a map from a constructor to its library builder because the builder
+ /// is used to report errors.
+ final Map<Constructor, LibraryBuilder> beingInferred = {};
final Instrumentation instrumentation;
@@ -248,20 +258,37 @@
}
/// Performs the third phase of top level inference, which is to visit all
- /// initializing formals and infer their types (if necessary) from the
- /// corresponding fields.
+ /// constructors still needing inference and infer the types of their
+ /// initializing formals from the corresponding fields.
void finishTopLevelInitializingFormals() {
- for (ShadowVariableDeclaration formal in initializingFormals) {
- try {
- formal.type = _inferInitializingFormalType(formal);
- } catch (e, s) {
- Location location = getLocationFromNode(formal);
- if (location == null) {
- rethrow;
- } else {
- throw new Crash(location.file, formal.fileOffset, e, s);
+ // Field types have all been inferred so there cannot be a cyclic
+ // dependency.
+ for (Constructor constructor in toBeInferred.keys) {
+ for (var declaration in constructor.function.positionalParameters) {
+ inferInitializingFormal(declaration, constructor);
+ }
+ for (var declaration in constructor.function.namedParameters) {
+ inferInitializingFormal(declaration, constructor);
+ }
+ }
+ toBeInferred.clear();
+ }
+
+ void inferInitializingFormal(VariableDeclaration formal, Constructor parent) {
+ if (formal.type == null) {
+ for (ShadowField field in parent.enclosingClass.fields) {
+ if (field.name.name == formal.name) {
+ if (field.inferenceNode != null) {
+ field.inferenceNode.resolve();
+ }
+ formal.type = field.type;
+ return;
}
}
+ // We did not find the corresponding field, so the program is erroneous.
+ // The error should have been reported elsewhere and type inference
+ // should continue by inferring dynamic.
+ formal.type = const DynamicType();
}
}
@@ -274,12 +301,6 @@
new TypeSchemaEnvironment(coreTypes, hierarchy, strongMode);
}
- /// Records that the given initializing [formal] will need top level type
- /// inference.
- void recordInitializingFormal(ShadowVariableDeclaration formal) {
- initializingFormals.add(formal);
- }
-
/// Records that the given static [field] will need top level type inference.
void recordStaticFieldInferenceCandidate(
ShadowField field, LibraryBuilder library) {
@@ -287,20 +308,4 @@
ShadowField.setInferenceNode(field, node);
staticInferenceNodes.add(node);
}
-
- DartType _inferInitializingFormalType(ShadowVariableDeclaration formal) {
- assert(ShadowVariableDeclaration.isImplicitlyTyped(formal));
- var enclosingClass = formal.parent?.parent?.parent;
- if (enclosingClass is Class) {
- for (var field in enclosingClass.fields) {
- if (field.name.name == formal.name) {
- return field.type;
- }
- }
- }
- // No matching field, or something else has gone wrong (e.g. initializing
- // formal outside of a class declaration). The error should be reported
- // elsewhere, so just infer `dynamic`.
- return const DynamicType();
- }
}
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart b/pkg/front_end/testcases/circularity-via-initializing-formal.dart
new file mode 100644
index 0000000..939cb91
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart
@@ -0,0 +1,18 @@
+// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This test is intended to trigger a circular top-level type-inference
+// dependency involving an initializing formal where the circularity is detected
+// when inferring the type of the constructor.
+//
+// The compiler should generate an error message and it should be properly
+// formatted including offset and lenght of the constructor.
+var x = new C._circular(null);
+
+class C {
+ var f = new C._circular(null);
+ C._circular(this.f);
+}
+
+main() {}
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.expect b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.expect
new file mode 100644
index 0000000..245a701
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.expect
@@ -0,0 +1,12 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C extends core::Object {
+ field dynamic f = new self::C::_circular(null);
+ constructor _circular(dynamic f) → void
+ : self::C::f = f, super core::Object::•()
+ ;
+}
+static field dynamic x = new self::C::_circular(null);
+static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.transformed.expect b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.transformed.expect
new file mode 100644
index 0000000..245a701
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.direct.transformed.expect
@@ -0,0 +1,12 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C extends core::Object {
+ field dynamic f = new self::C::_circular(null);
+ constructor _circular(dynamic f) → void
+ : self::C::f = f, super core::Object::•()
+ ;
+}
+static field dynamic x = new self::C::_circular(null);
+static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart.outline.expect b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.outline.expect
new file mode 100644
index 0000000..9bdea4e
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.outline.expect
@@ -0,0 +1,12 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C extends core::Object {
+ field dynamic f;
+ constructor _circular(dynamic f) → void
+ ;
+}
+static field dynamic x;
+static method main() → dynamic
+ ;
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.expect b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.expect
new file mode 100644
index 0000000..6fa3d2b
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.expect
@@ -0,0 +1,16 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C extends core::Object {
+ field self::C f = new self::C::_circular(null);
+ constructor _circular(self::C f) → void
+ : self::C::f = f, super core::Object::•()
+ ;
+}
+static field self::C x = new self::C::_circular(null);
+static const field dynamic #errors = const <dynamic>["pkg/front_end/testcases/circularity-via-initializing-formal.dart:15:3: Error: Can't infer the type of 'C._circular': circularity found during type inference.
+Specify the type explicitly.
+ C._circular(this.f);
+ ^^^^^^^^^^^"]/* from null */;
+static method main() → dynamic {}
diff --git a/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.transformed.expect b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.transformed.expect
new file mode 100644
index 0000000..6fa3d2b
--- /dev/null
+++ b/pkg/front_end/testcases/circularity-via-initializing-formal.dart.strong.transformed.expect
@@ -0,0 +1,16 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C extends core::Object {
+ field self::C f = new self::C::_circular(null);
+ constructor _circular(self::C f) → void
+ : self::C::f = f, super core::Object::•()
+ ;
+}
+static field self::C x = new self::C::_circular(null);
+static const field dynamic #errors = const <dynamic>["pkg/front_end/testcases/circularity-via-initializing-formal.dart:15:3: Error: Can't infer the type of 'C._circular': circularity found during type inference.
+Specify the type explicitly.
+ C._circular(this.f);
+ ^^^^^^^^^^^"]/* from null */;
+static method main() → dynamic {}
