From 41170b0a1576ab6a52bbd14f5792911089ed7895 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 7 Aug 2017 13:55:38 -0700 Subject: [PATCH] When reordering constructor initializers, use correct types for temp vars. In strong mode, when a call to a super-initializer is reordered, we can use the static type of the super-initializer arguments to set the types of the temporary variables that we use to do the reordering. This is desirable because it might help avoid unnecessary casts. In non-strong mode, we use `dynamic` for the temporary variables, to replicate Dart 1.0 behavior. R=scheglov@google.com Review-Url: https://codereview.chromium.org/2993193002 . --- .../lib/src/fasta/kernel/body_builder.dart | 4 +- .../kernel/kernel_procedure_builder.dart | 14 +++++-- .../src/fasta/kernel/kernel_shadow_ast.dart | 4 ++ .../lib/src/fasta/source/diet_listener.dart | 4 +- .../type_inference/type_inference_engine.dart | 4 ++ .../fasta/type_inference/type_inferrer.dart | 10 ++++- pkg/front_end/testcases/ast_builder.status | 1 + pkg/front_end/testcases/reorder_super.dart | 39 +++++++++++++++++++ .../reorder_super.dart.direct.expect | 33 ++++++++++++++++ .../reorder_super.dart.outline.expect | 22 +++++++++++ .../reorder_super.dart.strong.expect | 33 ++++++++++++++++ 11 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 pkg/front_end/testcases/reorder_super.dart create mode 100644 pkg/front_end/testcases/reorder_super.dart.direct.expect create mode 100644 pkg/front_end/testcases/reorder_super.dart.outline.expect create mode 100644 pkg/front_end/testcases/reorder_super.dart.strong.expect diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart index c821c293bb6..020c80b5986 100644 --- a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart @@ -511,7 +511,7 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { initializer = buildFieldInitializer(true, formal.name, formal.charOffset, new VariableGet(formal.declaration)); } - member.addInitializer(initializer); + member.addInitializer(initializer, _typeInferrer); } } } @@ -573,7 +573,7 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { } _typeInferrer.inferInitializer(initializer); if (member is KernelConstructorBuilder && !member.isExternal) { - member.addInitializer(initializer); + member.addInitializer(initializer, _typeInferrer); } else { deprecated_addCompileTimeError( token.charOffset, "Can't have initializers: ${member.name}"); 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 5da69b66712..81175a2893e 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 @@ -4,6 +4,9 @@ library fasta.kernel_procedure_builder; +import 'package:front_end/src/fasta/type_inference/type_inferrer.dart' + show TypeInferrer; + import 'package:kernel/ast.dart' show Arguments, @@ -389,7 +392,7 @@ class KernelConstructorBuilder extends KernelFunctionBuilder { } } - void addInitializer(Initializer initializer) { + void addInitializer(Initializer initializer, TypeInferrer typeInferrer) { List initializers = constructor.initializers; if (initializer is SuperInitializer) { checkSuperOrThisInitializer(initializer); @@ -420,8 +423,13 @@ class KernelConstructorBuilder extends KernelFunctionBuilder { Arguments arguments = superInitializer.arguments; List positional = arguments.positional; for (int i = 0; i < positional.length; i++) { - VariableDeclaration variable = - new VariableDeclaration.forValue(positional[i], isFinal: true); + var type = typeInferrer.typeSchemaEnvironment.strongMode + ? positional[i].getStaticType(typeInferrer.typeSchemaEnvironment) + : const DynamicType(); + VariableDeclaration variable = new VariableDeclaration.forValue( + positional[i], + isFinal: true, + type: type); initializers .add(new LocalInitializer(variable)..parent = constructor); positional[i] = new VariableGet(variable)..parent = arguments; 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 af7fb715430..60ed521aa41 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 @@ -2111,6 +2111,10 @@ class KernelTypeInferenceEngine extends TypeInferenceEngineImpl { return accessorNode; } + @override + TypeInferrer createDisabledTypeInferrer() => + new TypeInferrerDisabled(typeSchemaEnvironment); + @override KernelTypeInferrer createLocalTypeInferrer( Uri uri, TypeInferenceListener listener, InterfaceType thisType) { diff --git a/pkg/front_end/lib/src/fasta/source/diet_listener.dart b/pkg/front_end/lib/src/fasta/source/diet_listener.dart index 31d4daa38e0..e4ecf54f6b5 100644 --- a/pkg/front_end/lib/src/fasta/source/diet_listener.dart +++ b/pkg/front_end/lib/src/fasta/source/diet_listener.dart @@ -36,8 +36,6 @@ import '../type_inference/type_inference_engine.dart' show TypeInferenceEngine; import '../type_inference/type_inference_listener.dart' show TypeInferenceListener; -import '../type_inference/type_inferrer.dart' show TypeInferrerDisabled; - import '../util/link.dart' show Link; import 'source_library_builder.dart' show SourceLibraryBuilder; @@ -434,7 +432,7 @@ class DietListener extends StackListener { thisType = cls.thisType; } var typeInferrer = library.disableTypeInference - ? new TypeInferrerDisabled() + ? typeInferenceEngine.createDisabledTypeInferrer() : typeInferenceEngine.createLocalTypeInferrer(uri, listener, thisType); return new BodyBuilder(library, builder, memberScope, formalParameterScope, hierarchy, coreTypes, currentClass, isInstanceMember, uri, typeInferrer) 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 5ed1792a1b2..0ac33b1cfa5 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 @@ -140,6 +140,10 @@ abstract class TypeInferenceEngine { TypeInferrer createLocalTypeInferrer( Uri uri, TypeInferenceListener listener, InterfaceType thisType); + /// Creates a disabled type inferrer (intended for debugging and profiling + /// only). + TypeInferrer createDisabledTypeInferrer(); + /// Creates a [TypeInferrer] object which is ready to perform type inference /// on the given [field]. TypeInferrer createTopLevelTypeInferrer(TypeInferenceListener listener, diff --git a/pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart b/pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart index 8bec7c1b883..e9b22edb2eb 100644 --- a/pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart +++ b/pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart @@ -3,9 +3,9 @@ // BSD-style license that can be found in the LICENSE.md file. import 'package:front_end/src/base/instrumentation.dart'; -import 'package:front_end/src/fasta/problems.dart' show unhandled; import 'package:front_end/src/fasta/kernel/kernel_shadow_ast.dart'; import 'package:front_end/src/fasta/names.dart' show callName; +import 'package:front_end/src/fasta/problems.dart' show unhandled; import 'package:front_end/src/fasta/type_inference/type_inference_engine.dart'; import 'package:front_end/src/fasta/type_inference/type_inference_listener.dart'; import 'package:front_end/src/fasta/type_inference/type_promotion.dart'; @@ -203,6 +203,9 @@ abstract class TypeInferrer { /// this method body or initializer. TypePromoter get typePromoter; + /// Gets the [TypeSchemaEnvironment] being used for type inference. + TypeSchemaEnvironment get typeSchemaEnvironment; + /// The URI of the code for which type inference is currently being /// performed--this is used for testing. String get uri; @@ -233,6 +236,11 @@ class TypeInferrerDisabled extends TypeInferrer { @override final typePromoter = new TypePromoterDisabled(); + @override + final TypeSchemaEnvironment typeSchemaEnvironment; + + TypeInferrerDisabled(this.typeSchemaEnvironment); + @override String get uri => null; diff --git a/pkg/front_end/testcases/ast_builder.status b/pkg/front_end/testcases/ast_builder.status index 94e9ce166a5..163e22da8ef 100644 --- a/pkg/front_end/testcases/ast_builder.status +++ b/pkg/front_end/testcases/ast_builder.status @@ -427,6 +427,7 @@ regress/issue_29981: Crash regress/issue_29984: Crash regress/issue_29985: Crash regress/issue_29987: Crash +reorder_super: Crash static_setter: Crash store_load: Crash stringliteral: Crash diff --git a/pkg/front_end/testcases/reorder_super.dart b/pkg/front_end/testcases/reorder_super.dart new file mode 100644 index 00000000000..196c06141f6 --- /dev/null +++ b/pkg/front_end/testcases/reorder_super.dart @@ -0,0 +1,39 @@ +// Copyright (c) 2017, 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 verifies that super calls get reordered properly. It exercises the +// case where the arguments to super have a type other than `dynamic`. +String events = ''; + +int f(x) { + events += 'f($x)\n'; + return 0; +} + +String g(x) { + events += 'g($x)\n'; + return 'foo'; +} + +class B { + num x; + String y; + B(this.x, this.y) { + events += 'super($x, $y)\n'; + } +} + +class C extends B { + final z; + C() + : super(f(1), g(2)), + z = f(3); +} + +main() { + new C(); + if (events != 'f(1)\ng(2)\nf(3)\nsuper(0, foo)\n') { + throw 'Unexpected sequence of events: $events'; + } +} diff --git a/pkg/front_end/testcases/reorder_super.dart.direct.expect b/pkg/front_end/testcases/reorder_super.dart.direct.expect new file mode 100644 index 00000000000..9fa553f90ea --- /dev/null +++ b/pkg/front_end/testcases/reorder_super.dart.direct.expect @@ -0,0 +1,33 @@ +library; +import self as self; +import "dart:core" as core; + +class B extends core::Object { + field core::num x; + field core::String y; + constructor •(core::num x, core::String y) → void + : self::B::x = x, self::B::y = y, super core::Object::•() { + self::events = self::events.+("super(${this.x}, ${this.y})\n"); + } +} +class C extends self::B { + final field dynamic z; + constructor •() → void + : final dynamic #t1 = self::f(1), final dynamic #t2 = self::g(2), self::C::z = self::f(3), super self::B::•(#t1, #t2) + ; +} +static field core::String events = ""; +static method f(dynamic x) → core::int { + self::events = self::events.+("f(${x})\n"); + return 0; +} +static method g(dynamic x) → core::String { + self::events = self::events.+("g(${x})\n"); + return "foo"; +} +static method main() → dynamic { + new self::C::•(); + if(!self::events.==("f(1)\ng(2)\nf(3)\nsuper(0, foo)\n")) { + throw "Unexpected sequence of events: ${self::events}"; + } +} diff --git a/pkg/front_end/testcases/reorder_super.dart.outline.expect b/pkg/front_end/testcases/reorder_super.dart.outline.expect new file mode 100644 index 00000000000..947f8d25c60 --- /dev/null +++ b/pkg/front_end/testcases/reorder_super.dart.outline.expect @@ -0,0 +1,22 @@ +library; +import self as self; +import "dart:core" as core; + +class B extends core::Object { + field core::num x; + field core::String y; + constructor •(core::num x, core::String y) → void + ; +} +class C extends self::B { + final field dynamic z; + constructor •() → void + ; +} +static field core::String events; +static method f(dynamic x) → core::int + ; +static method g(dynamic x) → core::String + ; +static method main() → dynamic + ; diff --git a/pkg/front_end/testcases/reorder_super.dart.strong.expect b/pkg/front_end/testcases/reorder_super.dart.strong.expect new file mode 100644 index 00000000000..dbd74576e4e --- /dev/null +++ b/pkg/front_end/testcases/reorder_super.dart.strong.expect @@ -0,0 +1,33 @@ +library; +import self as self; +import "dart:core" as core; + +class B extends core::Object { + field core::num x; + field core::String y; + constructor •(core::num x, core::String y) → void + : self::B::x = x, self::B::y = y, super core::Object::•() { + self::events = self::events.{core::String::+}("super(${this.{self::B::x}}, ${this.{self::B::y}})\n"); + } +} +class C extends self::B { + final field dynamic z; + constructor •() → void + : final core::int #t1 = self::f(1), final core::String #t2 = self::g(2), self::C::z = self::f(3), super self::B::•(#t1, #t2) + ; +} +static field core::String events = ""; +static method f(dynamic x) → core::int { + self::events = self::events.{core::String::+}("f(${x})\n"); + return 0; +} +static method g(dynamic x) → core::String { + self::events = self::events.{core::String::+}("g(${x})\n"); + return "foo"; +} +static method main() → dynamic { + new self::C::•(); + if(!self::events.{core::String::==}("f(1)\ng(2)\nf(3)\nsuper(0, foo)\n")) { + throw "Unexpected sequence of events: ${self::events}"; + } +}