Migrate VM libraries to NNBD

[leafpetersen]

This tracks the migration of the VM specific platform libraries to NNBD. Specifically, this covers the following libraries:

• cli, ffi, developer, io, isolate, mirrors, profiler, vmservice

[leafpetersen]

@a-siva Is this on your radar? I think this makes most sense for the VM team to handle.

cc @lrhn

[a-siva]

I had not considered this yet, I will have to add it to our list of things to do for nnbd.
Do you have a recipe for how libraries are being migrated to NNBD so I can have somebofy start on this in Q4.

[leafpetersen]

We will shortly land a copy of sdk/sdk as sdk/sdk_nnbd . Migration will happen in place there. When we flip the experiment flag on by default, we can then unfork by copying over sdk_nnbd to sdk. In the meantime, we need to be be sure that any non-nnbd related changes go into both copies.

The migration itself might take advantage of the analyzer's migration tool where possible, but will also require somebody to just look at the APIs and make decisions about where to allow nullability, and then propagate that through the code.

cc @stereotype441

[lrhn]

One important caveat is that the VM's patch files do not live in sdk/.
It might be a good idea to move them (and any other Dart code that the VM depends on) to be under the sdk/ directory, so that we can have two versions of those files as well.

[leafpetersen]

@sortie I heard indirectly that you might be considering taking on io, is that correct?

[sortie]

@leafpetersen No, this is the first I hear about this. I did talk about Bob about migrating benchmarks, but that is another issue. I do currently own dart:io.

What would be involved in migrating such a library, or reviewing such changes?

[leafpetersen]

Ah, maybe I was just told that you were taking over dart:io, and hence you might be the right person to talk to.

In any case, someone needs to migrate the copy of dart:io in sdk_nnbd. This means basically two things: first making the API choices about which things should be nullable, and secondly migrating the code to be null clean. I could show you how to run the analyzer on the code with the experiment enabled.

[dcharkes]

I can take care of dart:ffi, most of the stuff should be not-nullable.

However, I'd like to only get started on this once we have some automated testing in place. Please keep me updated on the testing of the sdk_nnbd folder.

[sortie]

Hi @leafpetersen, can you describe the steps needed to migrate dart:io to nnbd?

[sortie]

Ping @leafpetersen

[leafpetersen]

Thanks for looking into this!

The plan for the dart:core libraries is to go with the following workflow:

• Migrate the libraries to pass the static analyzer (leave the patch files to the relevant teams)
• Migrate the core library tests to statically analyze (fix any issues from step 1)
• Get the core library tests running on each platform
  • This is blocked for each platform until the relevant team implements runtime support and migrates their patch files

I'd suggest that you might want to follow the same workflow. With that in mind, the first step is to make the API choices about what parameters/return types should be nullable vs non-nullable, and to get the code passing the static analyzer. You can run the analyzer on core libraries as follows:

xcodebuild/ReleaseX64/dart-sdk/bin/dartanalyzer --dart-sdk=/Users/leafp/src/dart-repo/sdk/sdk_nnbd/ --sdk-warnings --enable-experiment=non-nullable /Users/leafp/src/dart-repo/sdk/sdk_nnbd/lib/io

You will need to remove the // @ dart = 2.5 comments from the files in io to opt them into NNBD. Then you should see a mess of static errors from the above. :) At that point, you need to:

• make choices for the APIs (does this method accept int or int?)?
• fix the method bodies to pass the analyzer

Some example CLs showing how other core libraries were migrated:

• https://dart-review.googlesource.com/c/sdk/+/123621
• https://dart-review.googlesource.com/c/sdk/+/123600

Feel free to ping me, @munificent or @lrhn with questions about the NNBD features or whatever. Reviews can probably also be sent our way.

[leafpetersen]

The current open list is:

lib/isolate
lib/wasm
lib/io
lib/cli
lib/mirrors
lib/vmservice
lib/ffi

I believe that @sortie owns io, @dcharkes can you start on ffi? The test infrastructure is in place, you will probably need to port your own tests though. @munificent can help with the process.

[leafpetersen]

Migration instructions here.

[munificent]

There are subissues here for the various VM libraries, but it's not clear how those libraries map to tests in the "tests" directory. Can someone confirm that these test directories are all fully covered by this issue:

standalone_2/
standalone_2/deferred
standalone_2/http_launch_data
standalone_2/io
standalone_2/package

We are migrating tests by forking all of them which means it's possible for tests to slip through the cracks if no one owns a directory. Will the VM team claim all of those?

[a-siva]

This is done, we have switched all the libraries to the NNBD versions in the NNBD sdk builds.