Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Would like support for a VM Service query for null-safety mode (mixed, sound) #43588

Closed
jonahwilliams opened this issue Sep 28, 2020 · 7 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@jonahwilliams
Copy link
Contributor

The flutter tool would like to perform analytics tracking of hot reload performance based on mixed vs sound. The tool can sort of guestimate sound-ness by checking command line flags and parsing the entrypoint language version, or by performing an expression evaluation - but since the VM should already know it would be great if this could be surfaced through the VM Service API. It would be great if dwds could also support this query, too (@grouma ). FYI @jacob314 might have needed this too.

@bkonyi

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Sep 28, 2020
@a-siva a-siva added this to Needs triage in Dart VM support for Null Safe feature via automation Sep 28, 2020
dart-bot pushed a commit that referenced this issue Oct 1, 2020
Flags that can be set in `Dart_IsolateFlags` are now surfaced via the
Isolate JSON object returned from the service protocol. Currently only
returns boolean flags.

Related: #43588
Change-Id: I54a30155778de8612105eb4a5f60f8a222a086b4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165403
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Dart VM support for Null Safe feature automation moved this from Needs triage to Done Oct 7, 2020
@bkonyi
Copy link
Contributor

bkonyi commented Oct 7, 2020

Both the --enable-experiments and --strong-null-safety flags should appear in the FlagList returned by getFlagList now.

@jonahwilliams
Copy link
Contributor Author

I'm not seeing sound-null-safety set to true. I tried reading values from the flag list using hello_world.

Step 1: update examples/hello_world

diff --git a/examples/hello_world/lib/main.dart b/examples/hello_world/lib/main.dart
index 3cefcf3662..a3a48e6e10 100644
--- a/examples/hello_world/lib/main.dart
+++ b/examples/hello_world/lib/main.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// @dart = 2.11
 import 'package:flutter/widgets.dart';
 
 void main() =>

Step 2: print out flag value during flutter run

diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
index ac8c6ac639..f3d141ee47 100644
--- a/packages/flutter_tools/lib/src/resident_runner.dart
+++ b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -513,6 +513,11 @@ class FlutterDevice {
 
   Future<void> initLogReader() async {
     final vm_service.VM vm = await vmService.getVM();
+    final vm_service.FlagList flagList = await vmService.getFlagList();
+    final vm_service.Flag soundNullSafety = flagList.flags.firstWhere((vm_service.Flag flag) {
+      return flag.name == 'sound_null_safety';
+    }, orElse: () => null);
+    print('IS SOUND NULL SAFETY ENABLED: ${soundNullSafety?.valueAsString}|${soundNullSafety?.modified}');
     final DeviceLogReader logReader = await device.getLogReader(app: package);
     logReader.appPid = vm.pid;
   }

Step 3:

flutter run --sound-null-safety --enable-experiment=non-nullable
...
IS SOUND NULL SAFETY ENABLED: false|false

@grouma
Copy link
Member

grouma commented Oct 23, 2020

For the web we explicitly return an empty array. @jakemac53 and @annagrin is there a way to tell when the Frontend Server / build_web_compilers is run with null safety? We could even do an evaluation on the running app if necessary. Maybe @nshahan has some ideas.

@nshahan
Copy link
Contributor

nshahan commented Oct 23, 2020

Edited to reflect the way this works now. I forgot we made some changes here to the bootstrapping.

It seems like it would make more sense to retrieve the runtime mode elsewhere before the app starts running because we actually need to know and set the mode before we run main() and chose the right dart_sdk.js file.

That said the old standby test is [1, 2, null] is List<int> should be true in weak more and false in sound mode.

@grouma
Copy link
Member

grouma commented Oct 23, 2020

Is this something we should / could add in the DDC metadata?

@nshahan
Copy link
Contributor

nshahan commented Oct 23, 2020

I was talking with @annagrin about this topic earlier. She is investigating some refactoring of the infrastructure that needs to happen so the expression evaluation can use the correct mode. The case is a little different between frontend_server and build_web_compilers.

In the build_web_compilers case DDC could probably deduce the mode based on the dart sdk .dill file that is provided when compiling modules. As long as everything is working correctly the sdk .dill mode should match the version of the dart_sdk.js that is used when running. We could emit the mode in every metadata file.

I don't know exactly what happens in the frontend server setup or what emits the metadata file. Does this still hit the code path in DDC that writes the metadata file?

@grouma
Copy link
Member

grouma commented Oct 23, 2020

Does this still hit the code path in DDC that writes the metadata file?

That is my understanding. @annagrin would know best though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Development

No branches or pull requests

5 participants