diff --git a/pkg/vm/bin/kernel_service.dart b/pkg/vm/bin/kernel_service.dart index 8cb29dc2f54..14c33e5d0b5 100644 --- a/pkg/vm/bin/kernel_service.dart +++ b/pkg/vm/bin/kernel_service.dart @@ -643,7 +643,7 @@ Future _processExpressionCompilationRequest(request) async { port.send( new CompilationResult.errors([ "No platform found to initialize incremental compiler.", - ], null).toResponse(), + ]).toResponse(), ); return; } @@ -687,7 +687,7 @@ Future _processExpressionCompilationRequest(request) async { new CompilationResult.errors([ "Error when trying to create a compiler for expression compilation: " "'$e'.", - ], null).toResponse(), + ]).toResponse(), ); return; } @@ -698,7 +698,7 @@ Future _processExpressionCompilationRequest(request) async { port.send( new CompilationResult.errors([ "No incremental compiler available for this isolate.", - ], null).toResponse(), + ]).toResponse(), ); return; } @@ -724,9 +724,7 @@ Future _processExpressionCompilationRequest(request) async { ); if (procedure == null) { - port.send( - new CompilationResult.errors(["Invalid scope."], null).toResponse(), - ); + port.send(new CompilationResult.errors(["Invalid scope."]).toResponse()); return; } @@ -736,7 +734,7 @@ Future _processExpressionCompilationRequest(request) async { if (compiler.errorsPlain.isNotEmpty) { // TODO(sigmund): the compiler prints errors to the console, so we // shouldn't print those messages again here. - result = new CompilationResult.errors(compiler.errorsPlain, null); + result = new CompilationResult.errors(compiler.errorsPlain); } else { Component component = createExpressionEvaluationComponent(procedure); result = new CompilationResult.ok(serializeComponent(component)); @@ -899,7 +897,7 @@ Future _processLoadRequest(request) async { port.send( new CompilationResult.errors([ "No incremental compiler available for this isolate.", - ], null).toResponse(), + ]).toResponse(), ); return; } @@ -1024,19 +1022,7 @@ Future _processLoadRequest(request) async { ...(enableColors) ? compiler.errorsColorized : compiler.errorsPlain, ...nativeAssetsErrors.map((e) => e.message), ]; - final component = compilerResult.component; - if (component != null) { - result = new CompilationResult.errors( - errors, - serializeComponent( - component, - filter: (lib) => !loadedLibraries.contains(lib), - nativeAssetsComponent: nativeAssetsComponent, - ), - ); - } else { - result = new CompilationResult.errors(errors, null); - } + result = new CompilationResult.errors(errors); } else { // We serialize the component excluding vm_platform.dill because the VM has // these sources built-in. Everything loaded as a summary in @@ -1076,7 +1062,7 @@ Future _processLoadRequest(request) async { inputFileUri, inputFileUri, null, - new CompilationResult.errors(["unknown tag"], null).payload, + new CompilationResult.errors(["unknown tag"]).payload, ]); } } @@ -1288,8 +1274,7 @@ abstract class CompilationResult { factory CompilationResult.ok(Uint8List? bytes) = _CompilationOk; - factory CompilationResult.errors(List errors, Uint8List? bytes) = - _CompilationError; + factory CompilationResult.errors(List errors) = _CompilationError; factory CompilationResult.crash(Object exception, StackTrace stack) = _CompilationCrash; @@ -1332,10 +1317,9 @@ abstract class _CompilationFail extends CompilationResult { } class _CompilationError extends _CompilationFail { - final Uint8List? bytes; final List errors; - _CompilationError(this.errors, this.bytes); + _CompilationError(this.errors); @override Status get status => Status.error; @@ -1344,8 +1328,6 @@ class _CompilationError extends _CompilationFail { String get errorString => errors.join('\n'); String toString() => "_CompilationError(${errorString})"; - - List toResponse() => [status.index, payload, bytes]; } class _CompilationCrash extends _CompilationFail { diff --git a/pkg/vm/test/kernel_service_test.dart b/pkg/vm/test/kernel_service_test.dart index 30bb2e8a9c3..e3b7a806481 100644 --- a/pkg/vm/test/kernel_service_test.dart +++ b/pkg/vm/test/kernel_service_test.dart @@ -169,11 +169,10 @@ Future singleShotCompile( } return kernel_service.Status.ok; } else if (status == kernel_service.Status.error.index) { - expectLength(m, 3); + expectLength(m, 2); final String errors = m[1]; - final List bytes = m[2]; if (verbose) { - print("Compiled with errors --- $errors and ${bytes.length} bytes dill"); + print("Compiled with errors --- $errors"); } return kernel_service.Status.error; } else if (status == kernel_service.Status.crash.index) { diff --git a/runtime/bin/dfe.cc b/runtime/bin/dfe.cc index 9d4ce3314e9..8a10d19ce01 100644 --- a/runtime/bin/dfe.cc +++ b/runtime/bin/dfe.cc @@ -213,18 +213,15 @@ void DFE::CompileAndReadScript(const char* script_uri, *exit_code = 0; break; case Dart_KernelCompilationStatus_Error: - free(result.kernel); *error = result.error; // Copy error message. *exit_code = kCompilationErrorExitCode; break; case Dart_KernelCompilationStatus_Crash: - free(result.kernel); *error = result.error; // Copy error message. *exit_code = kDartFrontendErrorExitCode; break; case Dart_KernelCompilationStatus_Unknown: case Dart_KernelCompilationStatus_MsgFailed: - free(result.kernel); *error = result.error; // Copy error message. *exit_code = kErrorExitCode; break; diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index e0e6a63f244..848b1121837 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -8299,8 +8299,7 @@ TEST_CASE(DartAPI_Multiroot_Valid) { int sourcefiles_count = sizeof(sourcefiles) / sizeof(Dart_SourceFile); lib = TestCase::LoadTestScriptWithDFE( sourcefiles_count, sourcefiles, nullptr, /* finalize= */ true, - /* incrementally= */ true, /* allow_compile_errors= */ false, - "foo:///main.dart", + /* incrementally= */ true, "foo:///main.dart", /* multiroot_filepaths= */ "/bar,/baz", /* multiroot_scheme= */ "foo"); EXPECT_VALID(lib); @@ -8340,8 +8339,7 @@ TEST_CASE(DartAPI_Multiroot_FailWhenUriIsWrong) { int sourcefiles_count = sizeof(sourcefiles) / sizeof(Dart_SourceFile); lib = TestCase::LoadTestScriptWithDFE( sourcefiles_count, sourcefiles, nullptr, /* finalize= */ true, - /* incrementally= */ true, /* allow_compile_errors= */ false, - "foo1:///main.dart", + /* incrementally= */ true, "foo1:///main.dart", /* multiroot_filepaths= */ "/bar,/baz", /* multiroot_scheme= */ "foo"); EXPECT_ERROR(lib, diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index b639d6d38c6..05798636972 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -1184,9 +1184,6 @@ char* IsolateGroupReloadContext::CompileToKernel(bool force_reload, /*multiroot_scheme=*/nullptr); } if (retval.status != Dart_KernelCompilationStatus_Ok) { - if (retval.kernel != nullptr) { - free(retval.kernel); - } return retval.error; } *kernel_buffer = retval.kernel; diff --git a/runtime/vm/isolate_reload_test.cc b/runtime/vm/isolate_reload_test.cc index 7298f20550e..25670f50e2a 100644 --- a/runtime/vm/isolate_reload_test.cc +++ b/runtime/vm/isolate_reload_test.cc @@ -614,17 +614,16 @@ TEST_CASE(IsolateReload_LibraryImportAdded) { " return max(3, 4);\n" "}\n"; - const char* kReloadScript = + const char* kScript2 = "import 'dart:math';\n" "main() {\n" " return max(3, 4);\n" "}\n"; - Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript); - EXPECT_VALID(lib); - EXPECT_ERROR(SimpleInvokeError(lib, "main"), "max"); + Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr); + EXPECT_ERROR(lib, "Compilation failed"); - lib = TestCase::ReloadTestScript(kReloadScript); + lib = TestCase::LoadTestScript(kScript2, nullptr); EXPECT_VALID(lib); EXPECT_EQ(4, SimpleInvoke(lib, "main")); } @@ -1124,8 +1123,8 @@ TEST_CASE(IsolateReload_LibraryHide) { const char* kImportScript = "importedFunc() => 'a';\n"; TestCase::AddTestLib("test:lib1", kImportScript); - // Import 'test:lib1' with importedFunc hidden. Will result in an - // error. + // Import 'test:lib1' with importedFunc hidden. Will result in a + // compile-time error. const char* kScript = "import 'test:lib1' hide importedFunc;\n" "main() {\n" @@ -1134,18 +1133,17 @@ TEST_CASE(IsolateReload_LibraryHide) { // Dart_Handle result; - Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript); - EXPECT_VALID(lib); - EXPECT_ERROR(SimpleInvokeError(lib, "main"), "importedFunc"); + Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr); + EXPECT_ERROR(lib, "Compilation failed"); // Import 'test:lib1'. - const char* kReloadScript = + const char* kScript2 = "import 'test:lib1';\n" "main() {\n" " return importedFunc();\n" "}\n"; - lib = TestCase::ReloadTestScript(kReloadScript); + lib = TestCase::LoadTestScript(kScript2, nullptr); EXPECT_VALID(lib); EXPECT_STREQ("a", SimpleInvokeStr(lib, "main")); } @@ -1157,7 +1155,7 @@ TEST_CASE(IsolateReload_LibraryShow) { TestCase::AddTestLib("test:lib1", kImportScript); // Import 'test:lib1' with importedIntFunc visible. Will result in - // an error when 'main' is invoked. + // a compile-time error. const char* kScript = "import 'test:lib1' show importedIntFunc;\n" "main() {\n" @@ -1168,17 +1166,12 @@ TEST_CASE(IsolateReload_LibraryShow) { " return importedIntFunc();\n" "}\n"; - Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript); - EXPECT_VALID(lib); - - // Works. - EXPECT_EQ(4, SimpleInvoke(lib, "mainInt")); - // Results in an error. - EXPECT_ERROR(SimpleInvokeError(lib, "main"), "importedFunc"); + Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr); + EXPECT_ERROR(lib, "Compilation failed"); // Import 'test:lib1' with importedFunc visible. Will result in - // an error when 'mainInt' is invoked. - const char* kReloadScript = + // a compile-time error. + const char* kScript2 = "import 'test:lib1' show importedFunc;\n" "main() {\n" " return importedFunc();\n" @@ -1188,8 +1181,24 @@ TEST_CASE(IsolateReload_LibraryShow) { " return importedIntFunc();\n" "}\n"; - lib = TestCase::ReloadTestScript(kReloadScript); - EXPECT_ERROR(lib, "importedIntFunc"); + lib = TestCase::LoadTestScript(kScript2, nullptr); + EXPECT_ERROR(lib, "Compilation failed"); + + // Both imports 'test:lib1' are visible. + // Should be successful. + const char* kScript3 = + "import 'test:lib1' show importedFunc, importedIntFunc;\n" + "main() {\n" + " return importedFunc();\n" + "}\n" + "@pragma('vm:entry-point', 'call')\n" + "mainInt() {\n" + " return importedIntFunc();\n" + "}\n"; + + lib = TestCase::LoadTestScript(kScript3, nullptr); + EXPECT_VALID(lib); + EXPECT_EQ(4, SimpleInvoke(lib, "mainInt")); } // Verifies that we clear the ICs for the functions live on the stack in a way diff --git a/runtime/vm/kernel_isolate.cc b/runtime/vm/kernel_isolate.cc index b9cb9ac35bf..eda73cc8985 100644 --- a/runtime/vm/kernel_isolate.cc +++ b/runtime/vm/kernel_isolate.cc @@ -988,9 +988,6 @@ class KernelCompilationRequest : public ValueObject { if (result_.status == Dart_KernelCompilationStatus_Ok) { LoadKernelFromResponse(response[1]); } else { - if (result_.status == Dart_KernelCompilationStatus_Error) { - LoadKernelFromResponse(response[2]); - } // This is an error. ASSERT(response[1]->type == Dart_CObject_kString); result_.error = Utils::StrDup(response[1]->value.as_string); diff --git a/runtime/vm/source_report_test.cc b/runtime/vm/source_report_test.cc index 6be18d0a58e..330a1aa5866 100644 --- a/runtime/vm/source_report_test.cc +++ b/runtime/vm/source_report_test.cc @@ -10,15 +10,11 @@ namespace dart { #ifndef PRODUCT -static ObjectPtr ExecuteScript(const char* script, bool allow_errors = false) { +static ObjectPtr ExecuteScript(const char* script) { Dart_Handle lib; { TransitionVMToNative transition(Thread::Current()); - if (allow_errors) { - lib = TestCase::LoadTestScriptWithErrors(script, nullptr); - } else { - lib = TestCase::LoadTestScript(script, nullptr); - } + lib = TestCase::LoadTestScript(script, nullptr); EXPECT_VALID(lib); Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, nullptr); EXPECT_VALID(result); @@ -315,59 +311,6 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_UnusedClass_ForceCompile) { buffer); } -ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_UnusedClass_ForceCompileError) { - // WARNING: This MUST be big enough for the serialized JSON string. - const int kBufferSize = 1024; - char buffer[kBufferSize]; - const char* kScript = - "helper0() {}\n" - "class Unused {\n" - " helper1() { helper0()+ }\n" // syntax error - "}\n" - "main() {\n" - " helper0();\n" - "}"; - - Library& lib = Library::Handle(); - lib ^= ExecuteScript(kScript, true); - ASSERT(!lib.IsNull()); - const Script& script = - Script::Handle(lib.LookupScript(String::Handle(String::New("test-lib")))); - - SourceReport report(SourceReport::kCoverage, SourceReport::kForceCompile); - JSONStream js; - js.set_id_zone(thread->isolate()->EnsureDefaultServiceIdZone()); - report.PrintJSON(&js, script); - const char* json_str = js.ToCString(); - ASSERT(strlen(json_str) < kBufferSize); - ElideJSONSubstring("classes", json_str, buffer); - ElideJSONSubstring("libraries", buffer, buffer); - EXPECT_STREQ( - "{\"type\":\"SourceReport\",\"ranges\":[" - - // UnusedClass has a syntax error. - "{\"scriptIndex\":0,\"startPos\":30,\"endPos\":53,\"compiled\":false," - "\"error\":{\"type\":\"@Error\",\"_vmType\":\"LanguageError\"," - "\"kind\":\"LanguageError\",\"id\":\"objects\\/0\\/0\"," - "\"message\":\"'file:\\/\\/\\/test-lib': error: " - "\\/test-lib:3:26: " - "Error: This couldn't be parsed.\\n" - " helper1() { helper0()+ }\\n ^\"}}," - - // helper0 is compiled. - "{\"scriptIndex\":0,\"startPos\":0,\"endPos\":11,\"compiled\":true," - "\"coverage\":{\"hits\":[0],\"misses\":[]}}," - - // One range with two hits (main). - "{\"scriptIndex\":0,\"startPos\":57,\"endPos\":79,\"compiled\":true," - "\"coverage\":{\"hits\":[57,68],\"misses\":[]}}]," - - // Only one script in the script table. - "\"scripts\":[{\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\"," - "\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"}]}", - buffer); -} - ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_LibrariesAlreadyCompiled) { // WARNING: This MUST be big enough for the serialized JSON string. const int kBufferSize = 1024; diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index 23ed2b77324..c345f910bfb 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -299,7 +299,6 @@ char* TestCase::CompileTestScriptWithDFE(const char* url, const uint8_t** kernel_buffer, intptr_t* kernel_buffer_size, bool incrementally, - bool allow_compile_errors, const char* multiroot_filepaths, const char* multiroot_scheme) { // clang-format off @@ -313,8 +312,8 @@ char* TestCase::CompileTestScriptWithDFE(const char* url, // clang-format on return CompileTestScriptWithDFE( url, sizeof(sourcefiles) / sizeof(Dart_SourceFile), sourcefiles, - kernel_buffer, kernel_buffer_size, incrementally, allow_compile_errors, - multiroot_filepaths, multiroot_scheme); + kernel_buffer, kernel_buffer_size, incrementally, multiroot_filepaths, + multiroot_scheme); } char* TestCase::CompileTestScriptWithDFE(const char* url, @@ -323,7 +322,6 @@ char* TestCase::CompileTestScriptWithDFE(const char* url, const uint8_t** kernel_buffer, intptr_t* kernel_buffer_size, bool incrementally, - bool allow_compile_errors, const char* multiroot_filepaths, const char* multiroot_scheme) { Zone* zone = Thread::Current()->zone(); @@ -340,32 +338,26 @@ char* TestCase::CompileTestScriptWithDFE(const char* url, } } return ValidateCompilationResult(zone, result, kernel_buffer, - kernel_buffer_size, allow_compile_errors); + kernel_buffer_size); } char* TestCase::ValidateCompilationResult( Zone* zone, Dart_KernelCompilationResult compilation_result, const uint8_t** kernel_buffer, - intptr_t* kernel_buffer_size, - bool allow_compile_errors) { - if (!allow_compile_errors && - (compilation_result.status != Dart_KernelCompilationStatus_Ok)) { + intptr_t* kernel_buffer_size) { + if (compilation_result.status != Dart_KernelCompilationStatus_Ok) { + ASSERT(compilation_result.kernel == nullptr); char* result = OS::SCreate(zone, "Compilation failed %s", compilation_result.error); free(compilation_result.error); - if (compilation_result.kernel != nullptr) { - free(const_cast(compilation_result.kernel)); - } *kernel_buffer = nullptr; *kernel_buffer_size = 0; return result; } + ASSERT(compilation_result.error == nullptr); *kernel_buffer = compilation_result.kernel; *kernel_buffer_size = compilation_result.kernel_size; - if (compilation_result.error != nullptr) { - free(compilation_result.error); - } if (kernel_buffer == nullptr) { return OS::SCreate(zone, "front end generated a nullptr kernel file"); } @@ -425,25 +417,15 @@ static intptr_t BuildSourceFilesArray( return num_test_libs + 1; } -Dart_Handle TestCase::LoadTestScriptWithErrors( - const char* script, - Dart_NativeEntryResolver resolver, - const char* lib_url, - bool finalize_classes) { - return LoadTestScript(script, resolver, lib_url, finalize_classes, true); -} - Dart_Handle TestCase::LoadTestScript(const char* script, Dart_NativeEntryResolver resolver, const char* lib_url, - bool finalize_classes, - bool allow_compile_errors) { + bool finalize_classes) { LoadIsolateReloadTestLibIfNeeded(script); Dart_SourceFile* sourcefiles = nullptr; intptr_t num_sources = BuildSourceFilesArray(&sourcefiles, script, lib_url); - Dart_Handle result = - LoadTestScriptWithDFE(num_sources, sourcefiles, resolver, - finalize_classes, true, allow_compile_errors); + Dart_Handle result = LoadTestScriptWithDFE(num_sources, sourcefiles, resolver, + finalize_classes, true); delete[] sourcefiles; return result; } @@ -465,7 +447,7 @@ Dart_Handle TestCase::LoadTestLibrary(const char* lib_uri, char* error = TestCase::CompileTestScriptWithDFE( sourcefiles[0].uri, sourcefiles_count, sourcefiles, &kernel_buffer, &kernel_buffer_size, true); - if ((kernel_buffer == nullptr) && (error != nullptr)) { + if (error != nullptr) { return Dart_NewApiError(error); } @@ -492,7 +474,6 @@ Dart_Handle TestCase::LoadTestScriptWithDFE(int sourcefiles_count, Dart_NativeEntryResolver resolver, bool finalize, bool incrementally, - bool allow_compile_errors, const char* entry_script_uri, const char* multiroot_filepaths, const char* multiroot_scheme) { @@ -504,9 +485,8 @@ Dart_Handle TestCase::LoadTestScriptWithDFE(int sourcefiles_count, char* error = TestCase::CompileTestScriptWithDFE( entry_script_uri != nullptr ? entry_script_uri : sourcefiles[0].uri, sourcefiles_count, sourcefiles, &kernel_buffer, &kernel_buffer_size, - incrementally, allow_compile_errors, multiroot_filepaths, - multiroot_scheme); - if ((kernel_buffer == nullptr) && error != nullptr) { + incrementally, multiroot_filepaths, multiroot_scheme); + if (error != nullptr) { return Dart_NewApiError(error); } @@ -620,9 +600,6 @@ Dart_Handle TestCase::ReloadTestScript(const char* script) { if (compilation_result.status != Dart_KernelCompilationStatus_Ok) { Dart_Handle result = Dart_NewApiError(compilation_result.error); free(compilation_result.error); - if (compilation_result.kernel != nullptr) { - free(const_cast(compilation_result.kernel)); - } return result; } diff --git a/runtime/vm/unit_test.h b/runtime/vm/unit_test.h index 0bfc10f5219..0a282ccaf82 100644 --- a/runtime/vm/unit_test.h +++ b/runtime/vm/unit_test.h @@ -330,7 +330,6 @@ class TestCase : TestCaseBase { const uint8_t** kernel_buffer, intptr_t* kernel_buffer_size, bool incrementally = true, - bool allow_compile_errors = false, const char* multiroot_filepaths = nullptr, const char* multiroot_scheme = nullptr); static char* CompileTestScriptWithDFE( @@ -340,19 +339,12 @@ class TestCase : TestCaseBase { const uint8_t** kernel_buffer, intptr_t* kernel_buffer_size, bool incrementally = true, - bool allow_compile_errors = false, const char* multiroot_filepaths = nullptr, const char* multiroot_scheme = nullptr); static Dart_Handle LoadTestScript( const char* script, Dart_NativeEntryResolver resolver, const char* lib_uri = RESOLVED_USER_TEST_URI, - bool finalize = true, - bool allow_compile_errors = false); - static Dart_Handle LoadTestScriptWithErrors( - const char* script, - Dart_NativeEntryResolver resolver = nullptr, - const char* lib_uri = RESOLVED_USER_TEST_URI, bool finalize = true); static Dart_Handle LoadTestLibrary( const char* lib_uri, @@ -364,7 +356,6 @@ class TestCase : TestCaseBase { Dart_NativeEntryResolver resolver = nullptr, bool finalize = true, bool incrementally = true, - bool allow_compile_errors = false, const char* entry_script_uri = nullptr, const char* multiroot_filepaths = nullptr, const char* multiroot_scheme = nullptr); @@ -434,8 +425,7 @@ class TestCase : TestCaseBase { static char* ValidateCompilationResult(Zone* zone, Dart_KernelCompilationResult result, const uint8_t** kernel_buffer, - intptr_t* kernel_buffer_size, - bool allow_compile_errors); + intptr_t* kernel_buffer_size); RunEntry* const run_; };