Analyzer issue with complex generic functions

[kevmoo]

Expected: all good. Runs fine, even w/ --preview-dart-2
Actual:

  error • The argument type '(<S₀>(String, (Object) → S) → S₀) → Config' can't be assigned to the parameter type '(<S>(String, (Object) → S) → S) → Config' at lib/_testHarness.dart:7:23 • argument_type_not_assignable
  error • The return type 'int' isn't a 'S', as defined by anonymous closure at lib/_testHarness.dart:7:67 • return_of_invalid_type_from_closure

class Config {
  final int value;
  Config(this.value);
}

Config fromJson(Map json) =>
    checkMethod(json, (check) => new Config(check('value', (v) => v as int)));

typedef _CheckedConvert = S Function<S>(String key, _CastFunction<S>);

typedef _CastFunction<R> = R Function(Object);

T checkMethod<T>(
  Map map,
  T constructor(_CheckedConvert converter),
) {
  Q _checkedConvert<Q>(String key, _CastFunction<Q> castFunc) {
    return castFunc(map[key]);
  }

  return constructor(_checkedConvert);
}

CC @leafpetersen – I'll let you clean-up the labels

[leafpetersen]

Cut down example:

typedef _CheckedConvert = S Function<S>(String key, _CastFunction<S>);

typedef _CastFunction<R> = R Function(Object);

int Function(_CheckedConvert converter) f = (check) => 3;

[stereotype441]

I'm able to reproduce this bug. Having a look now.

[kevmoo]

Thanks!

…

On Mon, May 21, 2018 at 10:56 AM Paul Berry ***@***.***> wrote: I'm able to reproduce this bug. Having a look now. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#33159 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCih3aor4HjQRdjp9AhMOnnKJI4UVRks5t0v_YgaJpZM4UDjHY> .

[stereotype441]

This has been an extremely tricky bug, due to two design assumptions of the analyzer that no longer hold true in Dart 2.0:

• All function types are determined by starting with the type of an element in the element model, and substituting for free type variables in that type. In other words, function types never need to be created "out of thin air" based simply on parameter types and return types. Technically this wasn't even true in Dart 1.0 since LUB computation required creating types out thin air. In Dart 2.0, there are more instances of needing to create types out of thin air (such as the manipulations performed by StrongTypeSystemImpl._substituteForUnknownType), and due to type inference, the types created out of thin air get stored in the element model where they are long-lived.
• The set of free type variables in a function type may be found by starting with associated element and following the chain of enclosingElement pointers, gathering up type parameters at each level of the element tree. This doesn't work because in many (perhaps all) cases where function types created out of thin air, it is difficult to find a good place in the element tree to attach the synthetic element to, so we just leave the synthetic element's enclosingElement pointer as null.

In Leaf's cut down example, the way the problem manifests is that type inference infers a type of <S>(String, (Object) → S) → S for the check parameter of the anonymous function, but this inference process requires creating the parameter type (Object) → S out of thin air. As a result, the type (Object) → S is disconnected from the element model, so when the analyzer later attempts to substitute other types for S, the parameter type (Object) → S is unaffected. This causes two follow-on problems:

• When the type of check is printed, the analyzer does trial substitutions to determine whether the string representation of the type will contain ambiguous references to free type varialbles, and adds subscripts if necessary to disambiguate. These trial substitutions go wrong, causing the type to be incorrectly printed as <S₀>(String, (Object) → S) → S₀.
• When the type of check is compared against the type of _CheckedConvert in order to determine if the assignment is valid, a fresh type variable is substituted for S on both sides of the comparison (in an effort to ensure that function types that differ only in the names of their type parameters will still compare equal). This substitution goes wrong, causing the analyzer to incorrectly conclude that the assignment is invalid.

It would be nice to rewrite FunctionTypeImpl to fix these assumptions, but that's a big job that will take time (in part because the assumptions are baked into the analyzer API). I'm currently trying to find a pragmatic way to get this bug fixed, and move FunctionTypeImpl a few steps toward a better implementation, without having to do a complete rewrite now.

[kevmoo]

WIP fix at https://dart-review.googlesource.com/c/sdk/+/59160

[stereotype441]

Fix is now ready for review

[leafpetersen]

Woot! Great to see this bug well fixed. The old Dart 1 function type representation has been plaguing us. I'm betting there are a number of other open issues I've filed over the past year or two that have been closed by this as well.