Inference should flow information between arguments in a generic function call

[leafpetersen]

EDIT by @jmesserly

Inference should flow information up/down between arguments in a generic function call. This will help inference common methods like Iterable.fold and Future.forEach.

---

(original description)
We currently do downwards and upwards inference independently. This gets us a fair bit, but we should eventually integrate the two, probably following the colored local type inference approach.

[leafpetersen]

@kevmoo This is the bug for integrating inference further to make things work for fold. Here's an example pulled over from dart-lang/sdk#25491 . Currently we give no error on this since we fail to infer a type for x and hence leave it as dynamic.

void main() {
  List<int> o;
  var x = o.fold(0, (x, y) => x + y);
  List<String> y = x;
}

[kevmoo]

@leafpetersen That doesn't quite hit the same thing I'm seeing

void main() {
  List<String> o;
  var x = o.fold({}, (x, y) {
    x[y] = 'thing';
    return x;
  }) as Map<String, String>;
  _printInt(x);
}

_printInt(Map<String, String> things) => print(things);

on Dart VM version: 1.16.0-dev.0.0 (Wed Mar 9 15:04:40 2016) on "macos_x64"

[leafpetersen]

I think it does, but maybe I misunderstand. The redundant cast hint is a different issue - this is just about why it's not enough to put a type on the map literal. In your example, the cast gives inference enough context to infer the type of fold (and then you get the redundant cast warning). We ought to be able to infer the type just from the type of the receiver and the first argument - so putting a type annotation on the map literal should be enough here. It's not though - hence this bug.

Workarounds should be either typing both the map literal and the first argument to the lambda, or putting a type annotation x instead of using var.

[nex3]

@leafpetersen asked me to add this example:

/// Creates a new map from [map] with new keys and values.
///
/// The return values of [keyFn] are used as the keys and the return values of
/// [valueFn] are used as the values for the new map.
Map/*<K2, V2>*/ mapMap/*<K1, V1, K2, V2>*/(Map/*<K1, V1>*/ map,
    /*=K2*/ keyFn(/*=K1*/ key, /*=V1*/ value),
    /*=V2*/ valueFn(/*=K1*/ key, /*=V1*/ value)) =>
  new Map.fromIterable(map.keys,
      key: (key) => keyFn(key as dynamic/*=K1*/, map[key]),
      value: (key) => valueFn(key as dynamic/*=K1*/, map[key]));

/// Creates a new map from [map] with the same keys.
///
/// The return values of [fn] are used as the values for the new map.
Map/*<K, V2>*/ mapMapValues/*<K, V1, V2>*/(Map/*<K, V1>*/ map,
    /*=V2*/ fn(/*=K*/ key, /*=V1*/ value)) =>
  // [warning] Unsound implicit cast from dynamic to K.
  mapMap(map, (key, _) => key, fn);

in mapMapValues(), I'd expect inference to be able to tell that (key, _) => key has a return type of K, but currently it cannot.

[jmesserly]

Actually, let me repurpose this bug with a new subject line :)

We did integrate upwards and downwards inference. However, it is not able to flow information between arguments on an argument list, as required for several of the examples here such as Iterable.fold

EDIT: also to fix this, we'll need to spec this for a future version Dart and implement it in the new front end

[kevmoo]

The fold case is painful. Is this really area-language?

[matanlurey]

area-pain

[nex3]

This will also become a lot more relevant if we start seriously pushing for --no-implicit-dynamic.

[jmesserly]

The fold case is painful. Is this really area-language?

Yeah it's area-language. We can't do big changes to the type system in Analyzer, because it's all migrated to the new front end (thanks to @stereotype441), and we'll need to spec out (at least informally) what the new inference algorithm should be and how the rules work. The current technique is insufficiently powerful to handle fold and similar APIs. We would need to adapt some ideas from other languages/papers (e.g. C#).

[natebosch]

Has anything changed here since the last update?
Is anything likely to change here?

[jmesserly]

Has anything changed here since the last update?
Is anything likely to change here?

This inference improvement (if it happens, which I hope it does) is post 2.0, so there won't be any changes until after that.

edit: tagged this as "feature request"

[leafpetersen]

Yeah, let's keep this open for now until we re-organize language requests. This is something I'd still like to try to do in a future inference revision.

[alexander-doroshko]

Also reported here: https://youtrack.jetbrains.com/issue/WEB-34819

[eernstg]

One more example where an issue in this area was reported, again using fold: dart-lang/sdk#36405.

[mnordine]

Just want to chime in here that I've ran into this unexpected behavior. I've been migrating a large code base to Dart 2 with implicit-casts: false, implicit-dynamic: false, and having to go in and manually adding the generic type hints in hundreds of spots is annoying indeed. Would love to see this fixed! I like using fold and this feels like paying a little tax on it each time.

[eernstg]

Here is another example: dart-lang/sdk#37247.

[mnordine]

@leafpetersen Is this more of an issue now with nnbd?

I'm not sure if this is applicable to this particular issue, but somebody posted the following example in Gitter, complaining of the lack of inference. I tried with nnbd and it results in a static error:

Iterable<R> map<T, R>(
  Iterable<T> iterable,
  R Function(T) transform,
) sync* {
  for (var item in iterable) {
    yield transform(item);
  }
}

void main() {
  var list = [1, 2, 3];
  var mappedList = map(list, (x) => x + 1); // `x` is inferred as `Object?`
  print(mappedList);
}

An expression whose value can be 'null' must be null-checked before it can be dereferenced - line 12

[leafpetersen]

@leafpetersen Is this more of an issue now with nnbd?

Yes, or no, depending how you look at it. nnbd will make this more statically visible, which has both upsides and downsides.

Your example used to work because x was typed as dynamic. This is nice, because the code in fact runs correctly. But it's also not nice because code that doesn't run correctly will also pass the static analysis.

void main() {
  var list = [1, 2, 3];
  var mappedList = map(list, (x) => x.foo); // `x` is inferred as `Object?`
  print(mappedList);
}

This variant of your main function will pass static analysis in pre-nnbd Dart, but blow up at runtime.

But I do expect that nnbd will raise the profile of this issue, yes.

[kasperpeulen]

I actually love that nnbd raises an error.
The pre nnbd variant way doesnt look sound at all, as it silently infers it to dynamic it seems.

But you will get more people complaining 😅. If something blows up at runtime, the programmer get blamed by their manager. When something doesnt compile that theoretically should, the language designers gets blamed.

[mnordine]

Sure, and I'd love inferring the anonymous function parameter even more, :)

[stereotype441]

This is fully implemented and enabled as of 0a92b0c3aa2e32924a04c7a12a544f98b611fea4. It was released to the dev channel in 2.18.0-150.0.dev.

[lulupointu]

@stereotype441 Thanks for this amazing update !

However I don't know if this is expected but the following does not work:

List<num> numList;
...
List<int> intList = [1, 2, 3];
numList = [
  intList.fold(0, (x, y) => x.bitLength + y), // Error: The getter 'bitLength' isn't defined for the type 'num'.
];

While the following does work:

List<int> l1 = [1, 2, 3];
List<double> l2 = [1.0, 2.0, 3.0];
final a = [
  l1.fold(0, (x, y) => x.bitLength + y),
  l2.fold(0.0, (x, y) => x + y),
];
print(a is List<num>); // true

[pedromassango]

@felangel can this prevent the need to specify the state when declaring a BlocBuilder widget?

From (current): BlocBuilder<MyCubit, MyState>(...)
To (new/wanted): BlocBuilder<MyCubit>(...)

[felangel]

@felangel can this prevent the need to specify the state when declaring a BlocBuilder widget?

From (current): BlocBuilder<MyCubit, MyState>(...) To (new/wanted): BlocBuilder<MyCubit>(...)

Unfortunately I don't think it does see #620

[eernstg]

I think it would be useful to clarify the situation around this comment. Here is a simpler example showing the same phenomenon as the example given there:

X f<X>(X x, X Function(X) g) => g(x);

void main() {
  var i = 1;
  num n = f(i, (j) => j.bitLength); // 'The getter `bitLength` isn't defined for the type `num`'.
}

In this case (and in the original example) there is no direct connection to the support for horizontal inference (which is the new feature that was introduced to resolve this issue). The reason why the inferred actual type argument to the invocation of f is num rather than int is that the context type is num. That is, this inference result is caused by declaring n as num n, rather than declaring it, e.g., as var n or int n.

This means that there is no reasonable way we could change horizontal inference such that these examples would infer int rather than num. So there is no reason to have that discussion here.

It is a separate discussion that Dart type inference gives priority to the context type, rather than choosing a type which is obtained by plain bottom-up inference. Check out the issues about inference in general, which currently shows at least four issues about how to handle context types. The standard example showing that the context type cannot be ignored in Dart could be this one:

// Assume that Dart would give priority to bottom-up inference,
// rather than giving priority to the context type.

void main() {
  List<num> nums = []; // Inferred as `<Never>[]`.
  nums.add(1); // Throws.
}