Probable dart2js bug in profile or release mode

[escamoteur]

Hi,

several users of my get_it package ran into an exception when running their Flutter app on web in release or profile mode.
The exception and the generated code don't make any sense to me.

Please see details at

fluttercommunity/get_it#238

[sigmundch]

/cc @fishythefish

I've been spending some time investigating this issue, and I have narrowed down the problem a bit. I believe this is a bug in the RTI optimization.

I'll continue working on reducing the test further. I've managed to get it working locally without flutter, but I need a bit more iteration to figure out a good workaround and a smaller repro.

[sigmundch]

Looking more closely, this seems related to the use of call methods in the application. If I explicitly call generic methods instead of using the callable interface, the error goes away.

[sigmundch]

@fishythefish - I was able to reduce this further, but I haven't yet tried ripping out the get_it dependency. This is however a pretty small program:

import 'package:get_it/get_it.dart';

abstract class ISecureStorage {}
class WebSecureStorage implements ISecureStorage {}

class AppSecureStorage implements ISecureStorage {
  final ISecureStorage _storage;
  AppSecureStorage(this._storage);
}

class Module  {
  Module(this._getIt);
  final GetIt _getIt;
  AppSecureStorage get appSecureStorage => AppSecureStorage(_getIt<ISecureStorage>());
}

void main() {
  final get = GetIt.instance;
  final libCommonModule = Module(get);
  get.registerFactory<ISecureStorage>(() => WebSecureStorage());
  libCommonModule.appSecureStorage;
}

Oddly enough, inlining the appSecureStorage getter by hand makes the error disappear.

[sigmundch]

(and similarly changing _getIt<ISecureStorage>() to invoke .get directly also fixes the issue: _getIt<ISecureStorage>.get())

[fishythefish]

@sigmundch I'll take a look when I get a minute. I'm not surprised that call support is broken - I've fixed multiple similar bugs in the past.

[sigmundch]

And here is a standalone repro:

abstract class GetIt {
  static final GetIt instance = F();
  registerFactory<T>(Function f);
  call<T>();
  get<T>();
}

class F implements GetIt {
  Map<Type, Factory> map = {};
  registerFactory<T>(Function f) => map[T] = Factory(f);
  call<T>() => get<T>();
  get<T>() => map[T]!.allocator();
}

class Factory {
  Function allocator;
  Factory(this.allocator);
}

abstract class ISecureStorage {}
class WebSecureStorage implements ISecureStorage {}

class AppSecureStorage implements ISecureStorage {
  final ISecureStorage _storage;
  AppSecureStorage(this._storage);
}

class Module  {
  Module(this._getIt);
  final GetIt _getIt;
  AppSecureStorage get appSecureStorage => AppSecureStorage(_getIt<ISecureStorage>());
}

void main() {
  final get = GetIt.instance;
  final libCommonModule = Module(get);
  get.registerFactory<ISecureStorage>(() => WebSecureStorage());
  libCommonModule.appSecureStorage;
}

Note that removing the GetIt interface also hides the issue.

[fishythefish]

Great, thanks so much, @sigmundch! Small repros go a long way whenever I'm debugging these issues.

[fishythefish]

Just started taking a look at this, but wanted to provide a quick update. It seems that although call<T> should take a type argument, we appear to always be supplying dynamic, which causes the map lookup to fail. (Observe that in Siggi's last example, if you change the type argument to registerFactory from ISecureStorage to dynamic, the program runs with no issues.)

call usually needs to be handled a bit specially and we've had other call-specific issues in the RTI-need computation before. I'll track down the missing code path and put up a fix as soon as I can.

[sigmundch]

slight simplification of the repro:

abstract class B {
  call<T>();
}

class C implements B {
  call<T>() => print(T);
}

abstract class A {}

class Wrapper {
  Wrapper(this.b);
  final B b;
}

void main() {
  B b = C();
  b<A>();             // prints A
  Wrapper(b).b<A>();  // should print A, but prints dynamic
}

[sigmundch]

ironically, changing the last line to:

  (Wrapper(b).b)<A>(); // prints A;

does work. So there may be something here that relates to how the call itself is represented by the CFE lowering.

[fishythefish]

@johnniwinther It seems that Dart2jsTarget.supportsExplicitGetterCalls (added here) is affecting the CFE lowering in this case. Changing it to true fixes the issue. There's a TODO saying that we need to fix JS-interop before we can make this change:

sdk/pkg/compiler/lib/src/kernel/dart2js_target.dart

Lines 88 to 91 in f15518d

	// TODO(johnniwinther,sigmund): Remove this when js-interop handles getter
	// calls encoded with an explicit property get or disallows getter calls.
	@override
	bool get supportsExplicitGetterCalls => false;

In the meantime, is there a way we can get the explicit call in non-interop cases?

[johnniwinther]

I'll look into it.