Enable tree shaking of platform-specific code in AOT mode.

[mraleph]

Based on the converstation with Flutter users it seems that Flutter code it is common to guard large chunks of code behind runtime checks that boil down to:

if (Platform.isAndroid) {
  // ...
}

(actual flutter code might be using a layer on top: see https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/foundation/platform.dart)

Here's a list of subtasks:

• [x] #50472
• [ ] #50473
• [ ] #50474
• [ ] Eventually we may add annotation of platform-specific code + warning mechanism if not guarded by platform checks to Dart language tools (CFE, Analyzer)
• [ ] Investigate whether above tree shaking mechanisms are sufficient for flutter plugin code.

/cc @lukef @mit-mit

[mraleph]

/cc @sstrickl - also see other issues linked from the code.

[mkustermann]

Cross-linking some other issues

• https://github.com/flutter/flutter/issues/14233
• https://github.com/dart-lang/sdk/issues/35705
• https://github.com/dart-lang/language/issues/1889
• https://github.com/flutter/flutter/issues/113311

(related https://github.com/flutter/flutter/issues/99412)

See also go/flutter-platform-behavior

[mkustermann]

Assigning @sstrickl . Please work with @lrhn and @mit-mit on finding the right solution here.

[mkustermann]

The outcome of our offline discussion on Sept 29 was that we'll define a set of compiler-recognizable patterns for code that is dependent on platform/os/arch/... and then make our AOT compiler recognize those patterns to perform tree shaking on the kernel level.

@lrhn Have you thought about the set of patterns we should make our compiler support?

[lrhn]

My idea is fairly simple: https://gist.github.com/lrhn/2a3b06a8253fddaf61297cb604c6777f

Basically, recognize isLinux and operatingSystem == "linux", and combinations of those like isLinux || isWindows.

[mkustermann]

My idea is fairly simple: https://gist.github.com/lrhn/2a3b06a8253fddaf61297cb604c6777f

Basically, recognize isLinux and operatingSystem == "linux", and combinations of those like isLinux || isWindows.,

Thanks for writing it up, @lrhn !

What would be the code patterns we support on the usage site of those? (e.g. if (isLinux), caching isLinux in a local variable, using isLinux in other getters, more complicated control flow, ...)

Flutter is having it's own platform detection mechanism atm (it is exposing a defaultTargetPlatform getter that is based on _platform_io.dart which is currently even overridable at runtime by setting OS environment variable FLUTTER_TEST if assertions are enabled).

platform.TargetPlatform get defaultTargetPlatform {
  platform.TargetPlatform? result;
  if (Platform.isAndroid) {
    result = platform.TargetPlatform.android;
  } else if (Platform.isIOS) {
    ...
  }
  assert(() {
    if (Platform.environment.containsKey('FLUTTER_TEST')) {
      result = platform.TargetPlatform.android;
    }
    return true;
  }());
  if (platform.debugDefaultTargetPlatformOverride != null) {
    result = platform.debugDefaultTargetPlatformOverride;
  }
  if (result == null) throw ...;
  return result!;
}

Would we want to make our compiler recognize the above code to return a constant? Would we want to rewrite this implementation in flutter?

This flutter defaultTargetPlatform is used in practice to guard code, for example the flutter firebase plugin uses it here

[lrhn]

I'd prefer to not make things too complicated, so only direct occurrences of isLinux inside the condition expression itself. Remembering inferences from a boolean stored in a variable is possible (we've proven that for promotion), but it's also much more complicated and a little harder to read. The isX variables are short and directly useful.

I definitely don't want if (myPlatformTest()) { ... } to work, where myPlatformTest() => isLinux || isAndroid;. (If we ever make promoting methods work for type promotions, I'm willing to look at it again.)

As for more complicated control flow ... only if reasonable. (That would be recognizing escaping control flow.) It's safe to not start with it. The warnings apply whenever you use a platform specific method, unless we can deduce that you can only be on a supported platform (can't rule out that you are on an unsupported platform). Making our analysis better in the future will not introduce more errors, only fewer.

I'd probably make the compiler just check for the operatingSystem == "linux" (and "linux" == operatingSystem), and force-inline any getter starting with is from the package:platform/platform.dart library. Then we can add more isGroupOfPlatforms => isFoo || isBar || isBaz; getters if necessary without needing to add extra support.

We can choose to accept assert(isLinux); too, for warnings, even if we can't use it for optimizations when asserts are not enabled.

Flutter could rewrite their target platform code to use our getters. It won't make the result constant, but it might be known at compile-time anyway, if the compiler knows the value of, say, isAndroid. That's a valid optimization. Since they support overriding the actual runtime platform, it's different from what we do. The Dart compilers can safely optimize away non-platform code based on the values of the isX getters when it knows the value at runtime. The Flutter code above uses a non-const check for Platform.environment, so we can't do compile-time optimization based on the result of defaultTargetPlatform.

The goal here is two things:

• Primary: Get warnings if using platform-specific code when not definitely on that platform.
  • This is a static check.
  • This is best effort.
  • We can decide on any number of ways to decide the current platform. Better analysis only removes warnings.
  • We can even use assert(isLinux) as a sign that the following code is definitely running on Linux, even if it's not safe.
• Secondary: Optimize AoT tree-shaking by removing any code guarded by check for a platform other than the one being compiled for.
  • This too must be best effort.
  • It must be dynamically safe, so we can't rely on assert(isLinux) for truth in production, which is where you really want the tree-shaking.
  • It must not change program behavior, so we can't allow you to mock the current platform dynamically, it must at least go through the compiler.

If used correctly, all code that doesn't run on the current platform should be tree-shakable, because any use of it is definitely inside other code which can be tree-shakable.

[mkustermann]

Flutter could rewrite their target platform code to use our getters. It won't make the result constant, ...

We have to be able to tree shake code that is guarded by platform checks. If flutter provides a blessed API that they recommend to users, users will use it, so we have to be able to tree shake based on that API.

(Seemingly this was originally added in https://github.com/flutter/flutter/pull/33663)

[mkustermann]

Filed some sub-issues (see https://github.com/dart-lang/sdk/issues/31969#issue-290926253)