From e84bea25df23ea5f149fc808a6423c7500b9cace Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Thu, 23 Jan 2020 12:52:57 +0000 Subject: [PATCH] [CFE] Fix crash in incremental compiler caused by async redirecting factory Redirectig factories are encoded in a special way to "recover" their data (together with _redirecting# field). Before this CL, if a redirecting factory was async (that's an error) it was encoded so we couldn't load that dill again and that we would throw an error if we tried. This CL fixes that. Change-Id: If73ef5f662fb23bc083eb1af086d9d7694ca538c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133061 Reviewed-by: Johnni Winther Commit-Queue: Jens Johansen --- .../lib/src/fasta/source/diet_listener.dart | 10 +++- .../crash_test_1.yaml | 22 ++++++- .../crash_test_1.yaml.world.1.expect | 60 ++++--------------- .../crash_test_1.yaml.world.2.expect | 28 +++++++++ 4 files changed, 69 insertions(+), 51 deletions(-) create mode 100644 pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.2.expect 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 c3726219c6760..f5617496580d7 100644 --- a/pkg/front_end/lib/src/fasta/source/diet_listener.dart +++ b/pkg/front_end/lib/src/fasta/source/diet_listener.dart @@ -97,6 +97,7 @@ class DietListener extends StackListenerImpl { DeclarationBuilder _currentDeclaration; ClassBuilder _currentClass; + bool _inRedirectingFactory = false; bool currentClassIsParserRecovery = false; @@ -590,7 +591,7 @@ class DietListener extends StackListenerImpl { if (name is ParserRecovery || currentClassIsParserRecovery) return; FunctionBuilder builder = lookupConstructor(beginToken, name); - if (bodyToken == null || optional("=", bodyToken.endGroup.next)) { + if (_inRedirectingFactory) { buildRedirectingFactoryMethod( bodyToken, builder, MemberKind.Factory, metadata); } else { @@ -625,6 +626,7 @@ class DietListener extends StackListenerImpl { void endRedirectingFactoryBody(Token beginToken, Token endToken) { debugEvent("RedirectingFactoryBody"); discard(1); // ConstructorReference. + _inRedirectingFactory = true; } @override @@ -793,6 +795,7 @@ class DietListener extends StackListenerImpl { void endMember() { debugEvent("Member"); checkEmpty(-1); + _inRedirectingFactory = false; } @override @@ -933,6 +936,11 @@ class DietListener extends StackListenerImpl { token = parser.parseInitializersOpt(token); token = parser.parseAsyncModifierOpt(token); AsyncMarker asyncModifier = getAsyncMarker(listener) ?? AsyncMarker.Sync; + if (kind == MemberKind.Factory && asyncModifier != AsyncMarker.Sync) { + // Factories has to be sync. The parser issued an error. + // Recover to sync. + asyncModifier = AsyncMarker.Sync; + } bool isExpression = false; bool allowAbstract = asyncModifier == AsyncMarker.Sync; parser.parseFunctionBody(token, isExpression, allowAbstract); diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml index 10b94fd0e6ab9..1b6622896ba9f 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml @@ -9,13 +9,31 @@ worlds: - entry: main.dart sources: main.dart: | + import "lib.dart"; + void main() { + var c = new C.e4(); + } + lib.dart: | class C { C(); factory C.e4() async = C; } - + expectedLibraryCount: 2 + errors: true + - entry: main.dart + invalidate: + - main.dart + sources: + main.dart: | + import "lib.dart"; void main() { var c = new C.e4(); + print(c); + } + lib.dart: | + class C { + C(); + factory C.e4() async = C; } - expectedLibraryCount: 1 + expectedLibraryCount: 2 errors: true diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.1.expect b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.1.expect index 10227fce20574..bd5a25a79e7da 100644 --- a/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.1.expect +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.1.expect @@ -1,63 +1,27 @@ main = ; -library from "org-dartlang-test:///main.dart" as main { +library from "org-dartlang-test:///lib.dart" as lib { // // Problems in library: // -// org-dartlang-test:///main.dart:3:11: Error: Functions marked 'async' must have a return type assignable to 'Future'. -// factory C.e4() async = C; -// ^^ -// -// org-dartlang-test:///main.dart:3:18: Error: Factory bodies can't use 'async', 'async*', or 'sync*'. +// org-dartlang-test:///lib.dart:3:18: Error: Factory bodies can't use 'async', 'async*', or 'sync*'. // factory C.e4() async = C; // ^^^^^ -// -// org-dartlang-test:///main.dart:3:24: Error: Expected a function body or '=>'. -// Try adding {}. -// factory C.e4() async = C; -// ^ -// -// org-dartlang-test:///main.dart:3:26: Error: A value of type 'Type' can't be assigned to a variable of type 'FutureOr'. -// - 'Type' is from 'dart:core'. -// - 'FutureOr' is from 'dart:async'. -// - 'C' is from 'org-dartlang-test:///main.dart'. -// factory C.e4() async = C; -// ^ // class C extends dart.core::Object { - static field dynamic _redirecting# = [main::C::e4]; - constructor •() → main::C* + static field dynamic _redirecting# = [lib::C::e4]; + constructor •() → lib::C* : super dart.core::Object::•() ; - static factory e4() → main::C* /* originally async */ { - final dart.async::_AsyncAwaitCompleter* :async_completer = new dart.async::_AsyncAwaitCompleter::•(); - dart.async::FutureOr* :return_value; - dynamic :async_stack_trace; - dynamic :async_op_then; - dynamic :async_op_error; - dart.core::int* :await_jump_var = 0; - dynamic :await_ctx_var; - function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding - try { - #L1: - { - :return_value = let final #t1 = invalid-expression "org-dartlang-test:///main.dart:3:26: Error: A value of type 'Type' can't be assigned to a variable of type 'FutureOr'.\n - 'Type' is from 'dart:core'.\n - 'FutureOr' is from 'dart:async'.\n - 'C' is from 'org-dartlang-test:///main.dart'.\n factory C.e4() async = C;\n ^" in main::C* as{TypeError} dart.async::FutureOr*; - break #L1; - } - dart.async::_completeOnAsyncReturn(:async_completer, :return_value); - return; - } - on dynamic catch(dynamic :exception, dynamic :stack_trace) { - :async_completer.{dart.async::Completer::completeError}(:exception, :stack_trace); - } - :async_stack_trace = dart.async::_asyncStackTraceHelper(:async_op); - :async_op_then = dart.async::_asyncThenWrapperHelper(:async_op); - :async_op_error = dart.async::_asyncErrorWrapperHelper(:async_op); - :async_completer.start(:async_op); - return :async_completer.{dart.async::Completer::future}; - } + static factory e4() → lib::C* + let dynamic #redirecting_factory = lib::C::• in invalid-expression; } +} +library from "org-dartlang-test:///main.dart" as main { + + import "org-dartlang-test:///lib.dart"; + static method main() → void { - main::C* c = main::C::e4(); + lib::C* c = new lib::C::•(); } } diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.2.expect b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.2.expect new file mode 100644 index 0000000000000..8ef39f628966e --- /dev/null +++ b/pkg/front_end/testcases/incremental_initialize_from_dill/crash_test_1.yaml.world.2.expect @@ -0,0 +1,28 @@ +main = ; +library from "org-dartlang-test:///lib.dart" as lib { +// +// Problems in library: +// +// org-dartlang-test:///lib.dart:3:18: Error: Factory bodies can't use 'async', 'async*', or 'sync*'. +// factory C.e4() async = C; +// ^^^^^ +// + + class C extends dart.core::Object { + static field dynamic _redirecting# = [lib::C::e4]; + constructor •() → lib::C* + : super dart.core::Object::•() + ; + static factory e4() → lib::C* + let dynamic #redirecting_factory = lib::C::• in invalid-expression; + } +} +library from "org-dartlang-test:///main.dart" as main { + + import "org-dartlang-test:///lib.dart"; + + static method main() → void { + lib::C* c = new lib::C::•(); + dart.core::print(c); + } +}