Sdk: Dart2js release build may make generic parameters dynamic

[rrousselGit]

From https://github.com/rrousselGit/provider/issues/406

@mono0926 found a compilation issue with Dart2js when running:

flutter run -t chrome --release

where the build output behaves differently from debug builds and VM builds.

The issue happens with the following code:

typedef Locator = void Function<T>();

class Bar {
  Bar({this.read}) {
    read<int>();
  }

  final Locator read;
}

void main() {
  Bar(read: <T>() {
    print(T);
    return null;
  });
}

The expected behavior of this code is to print int in the console, which it does in debug builds and VM release builds.
But when running this code with flutter -t chrome --release, this will print dynamic

This causes InheritedWIdgets from Flutter to fail to be resolved

[rrousselGit]

This was tested on the up-to-date master channel of Flutter (1.18.0-5.0.pre-57), which uses Dart 2.8.0-dev.20.0

[vsmenon]

fyi - @rakudrama @fishythefish

[fishythefish]

Hmm, looks like we're discarding the type argument in the backend somewhere:

V.main_closure.prototype = {
  call$1$0: function($T) {
    H.printString(H.createRuntimeType($T).toString$0(0));
  },
  call$0: function() {
    return this.call$1$0(type$.dynamic);
  },
  $signature: 0
};

This is the case even if the Bar constructor is simply Bar(this.read); and main itself invokes .read<int>(), but the problem vanishes if the Locator is a local instead of a member.

[rakudrama]

At the very beginning the SSA code is missing the type argument.
With the command-line option --disable-rti-optimization, it prints int.
The type is probably being dropped on the basis of some 'rti-need' bug.

[sma]

This bug stopps us from release a web app. Is there any workaround other then removing Provider from the whole app?

[fishythefish]

Taking a deeper look at this now since this is causing NNBD tests to fail.

[whesse]

The fix for this is causing test failures on dartdevc firefox
https://dart-review.googlesource.com/c/sdk/+/151301
introduced failures on firefox:

dartdevk-checked-linux-release-firefox
Pass -> RuntimeError (expected Pass)
✔ language_2/regress/regress22443_test
Timeout -> RuntimeError (expected Pass)
✔ language_2/regress/regress23244_test

[fishythefish]

@whesse I'm a little confused by that, since this fix only touches pkg/compiler (the dart2js backend, not DDC), and the tests you mentioned don't seem to have anything to do with this issue.

The logs (22443, 23244) are inscrutable to me, so I don't know if this is just flakiness or not. @nshahan, do you know what's going on here?

[nshahan]

There are parts of the test infrastructure that get compiled by dart2js and then used to run tests for DDC so it is technically possible for a dart2js only change to break a DDC bot but that should appear to be an infra failure.

These failure logs show DDC stack traces so I'm not sure how your change could be the cause. @whesse Is it possible that they are flakes or did we update the version of firefox running on the bots around the same time this change landed?

[whesse]

I checked on it, by looking in the results.json for the build after the one they were reported on.
They were detected to be flaky in that next build, so they stopped being reported on the results feed, and the previous failure was not marked as "superceded" (inactive, replaced by a more recent change).

This failure has happened a number of times before, and confuses everyone and wastes their time.
The fix is to report a change in flaky status to the results feed, and mark earlier failures on now flaky tests as inactive.
I see now that fixing this is urgent, because it wastes so many users' time when it happens.
https://github.com/dart-lang/dart_ci/issues/98

[matthewtsmith]

I'm not sure when this will be available in the beta channel but for those using StateNotifier/Provider/LocatorMixin who need a workaround I was able to solve it by using the following:

  Repository get repository {
    var reader = read;
    return reader();
  }

As this comment mentions, making read a local var seems to resolve the issue.

[rrousselGit]

I'm not sure why this is closed as the bug is still happening on the latest version of Dart/Flutter

Dart SDK version: 2.9.0-21.0.dev.flutter-06cb010247 (be) (Tue Jul 7 08:14:51 2020 +0000) on "macos_x64"

[fishythefish]

My apologies - I closed the issue when my fix landed and our tests started passing, but it looks like there's still another bug lurking around when --omit-implicit-checks is passed. I'll add another test and investigate.

[fishythefish]

The initial fix for this was pretty quick, but revealed a deeper issue causing other tests (mainly async ones) to fail. I have a more complete fix that I'm working on landing now.

[ochmist]

Hello guys, any news on this? This is blocking us from releasing a web app on:

Flutter (Channel master, 1.21.0-10.0.pre.18, on Mac OS X 10.15.6 19G73, locale en-US)

Thanks in advance.

[sigmundch]

HI @ochmist - the fix landed in https://github.com/dart-lang/sdk/commit/5671730c5ae2e08bf8c0d63bb076fec1930ce868, which is part of Dart's 2.10.0-1.0.dev release and newer releases.

I am not sure on which flutter release this version was rolled into. When I try flutter doctor -v, I see already a newer version of the Dart sdk in the master channel:

[✓] Flutter (Channel master, 1.21.0-10.0.pre.41, on Linux, locale en_US.UTF-8)
    • Flutter version 1.21.0-10.0.pre.41 at /usr/local/google/home/sigmund/dart/flutter/flutter
    • Framework revision 49fac9a885 (2 days ago), 2020-08-11 11:44:03 -0700
    • Engine revision 9b4ce4e3a0
    • Dart version 2.10.0 (build 2.10.0-11.0.dev)

So the fix should be there. Is the Dart version in your case further behind? Is this resolved after upgrading to the latest flutter?

[ochmist]

Hi @sigmundch, Confirmed that this works well after flutter upgrade. Thank you very much.

[venkatd]

@ochmist what flutter version is working for you (flutter doctor result)? We are also using StateNotifier and our web app is crashing due to this bug.Thanks!

@matthewtsmith could you please elaborate on the change you made? I wasn't able to understand the suggestion and how to apply it to our code.

[ochmist]

Actually, after the upgrade it ended up working for a while but breaking soon after. I noticed that once of my dependencies was still outdated and the crash was happening in there.

[fishythefish]

To clarify, do you have a repro that still fails with the latest SDK, or is this just a toolchain issue?

[takeru]

I tried flutter web + riverpod.

This code doesn't work.

final xxxxProvider = StateNotifierProvider.autoDispose.family<FooController, Bar>((ref, foo) {

Then, I built without minify. https://stackoverflow.com/questions/59710360

flutter build web --profile --dart-define=Dart2jsOptimization=O0

I can see this log in chrome console.

Uncaught TypeError: Instance of 'AutoDisposeProviderElement<StateNotifier<dynamic>, StateNotifier<dynamic>>': type 'AutoDisposeProviderElement<StateNotifier<dynamic>, StateNotifier<dynamic>>' is not a subtype of type 'ProviderElement<Object, FooController>'

But this workaround saved me.
https://github.com/rrousselGit/river_pod/commit/5d6d1b1f74277a1609a2858459e0fdb7ca8a1723#diff-d98faf2ba46cc2fa0f6e56d1bf216afe6b380aabc34ba3b4f74ae7d83cecb52fR36

final $family = StateNotifierProvider.autoDispose.family;
final xxxxProvider = $family<FooController, Bar>((ref, foo) {