Does horizontal inference apply for record literals?

[eernstg]

Consider the following program:

T f<T, U, V>((void Function(T, U), T, U Function(V), V Function(U)) r) {
  print('$T, $U, $V');
  return r.$1;
}

void main() {
  double e = 1.0;
  e = f(((t, u) {}, 3, (v) => 'Hello', (u) => true)); // 'double, String, bool'?
}

[lrhn]

Example using fold:

extension <E> on Iterable<E> {
  T foldRec<T>((T value, T Function(T, E)) arguments) {
    var (value, function) = arguments;
    for (var e in this) value = function(value, e);
    return value;
  }
}

Use:

var x = [1,2,3].foldRec((0, (a, b) => a + b));

[stereotype441]

FWIW, after I implemented horizontal inference, and updated Google internal sources to no longer use explicit types in the places where horizontal inference made them unnecessary, I was surprised how infrequently it actually made a difference in real-world code. Honestly, if I had it to do all over again, I probably wouldn't have even bothered implementing the feature in the first place, and would have spent my time on a feature with a greater user benefit.

Based on how little code was affected by the full horizontal inference feature, I personally find it hard to believe that applying horizontal inference to record literals is ever going to make enough of a difference for anyone to care. I would rather save our limited implementation time for other features.

[leafpetersen]

I'd be very surprised if this is a common enough pattern for records to matter.

Honestly, if I had it to do all over again, I probably wouldn't have even bothered implementing the feature in the first place, and would have spent my time on a feature with a greater user benefit.

FWIW, this was a very common issue filed in the issue tracker after we rolled out null safety, so I suspect that even if the number of places it applied was small, the user visibility was relatively high. Also, are you sure you were capturing all places this applied? I currently see quite a lot of hits inside google for \.fold\< in null safe code, many of which I think are not now required because of your change.

[eernstg]

Very good, I think the answer is "No, horizontal inference does not apply for record literals". Any worries about that outcome?

[leafpetersen]

I'd like to hear from @munificent a bit. There's one use case here that worries me. There's a vision to be able to use spread operators on records to pass arguments, for UI as code applications. Given that, it would be a bit off if:

FooWidget(3, (x) => x+1)

worked fine, but

FooWidget(
  if (...)
      ...(3, (x) => x+1)
  else
     ...(4, (x) => x-1)
)

failed.

I do seem to recall that there are some flutter APIs that take callbacks where horizontal inference might apply.

[munificent]

I'd like to hear from @munificent a bit.

I'm, uh, definitely not familiar enough with this dark corner of the language to have an opinion one way or the other. I do know that the weak type inference on fold() was a real pain point and if "horizontal inference" is what fixed that, it sounds nice. :)

Given that, it would be a bit off if:

FooWidget(3, (x) => x+1)

worked fine, but

FooWidget(
  if (...)
      ...(3, (x) => x+1)
  else
     ...(4, (x) => x-1)
)

failed.

Agreed, though I don't know what the ramifications of wanting this to work are.

[munificent]

I'd like to resolve this issue one way or the other, but I don't have enough expertise to make the call. I find Leaf's point that spreading a record into an argument list should have the same inference experience as an in-place argument list very compelling. So given that, I'm inclined to say, yes, we do horizontal inference for records too.

But I don't know enough about this corner of the language and its implementation to know the consequences of that. @stereotype441, thoughts? Anyone else we can consult?

[leafpetersen]

Thinking about this a bit more, I'm not sure this question is even well-formed. What does it mean to do horizontal inference here? The basis of horizontal inference is that you have a set of parameters that are tied together by a type parameter. e.g. you have something like listOfInt.fold(3, (x, y) => x + y where T fold<T>(T unit, T Function(T, E) combine) and you heuristically order the arguments so that upwards inference on the first argument can provide a solution for T before you try to infer the lambda argument.

But for a record, there are no type arguments! In the example from @lrhn (var x = [1,2,3].foldRec((0, (a, b) => a + b));) the downwards context for (0, (a, b) => a + b)) will be something like (_, _ Function(_, int)). So there's no type arguments to use to tie together the two elements of the record. You'd have to somehow "reach out" to the outer context, which seems messy.

I have thought vaguely in the past about attaching more information to _ (e.g. I think it might be useful to attach a bound to it), so there may be a path where you recorded the fact that two _ parameters had to be the same thing. I worry that this gets you into general unification though.

So I'm not sure I see a path forward here. For the case of spreading a record into an argument list, I think that case specifically could possibly be handled by the existing horizontal inference. That is, when/if we add spread for records, we could specify inference for the spread operator to infer f(...(recordElements)) precisely as if it was f(recordElements).

[munificent]

You'd have to somehow "reach out" to the outer context, which seems messy.

I definitely don't understand all of Paul's horizontal inference proposal, but instead of reaching back out, I'm guessing that the way we'd model this is by having horizontal inference recurse into record literals (spread or not) that appear as arguments in function invocations. Then it would potentially split those record fields into separate stages so that the partial constraint solution for a type argument in the surrounding generic function being invoked can then be used as the context type for later fields in that record (or potentially other later record arguments, nested records, etc.).

In other words, for horizontal inference, it would conceptually flatten out the entire tree of record literals inside function arguments to something like a linear list of leaf arguments/record fields, and then do horizontal inference across that flattened list.

As I understand it, that would make Lasse's example work, and intuitively I would expect his example to work. It would feel weird if wrapping a couple of arguments in a record and changing the corresponding set of parameters to be a single parameter of a record type caused inference to get worse. Not, like, intolerably weird. But that intuitively feels to me like a transformation that should be transparent to the type system and should just work.

[leafpetersen]

In other words, for horizontal inference, it would conceptually flatten out the entire tree of record literals inside function arguments to something like a linear list of leaf arguments/record fields, and then do horizontal inference across that flattened list.

That's fair, I think you could make sense of that. It's basically doing what I proposed for spreads of records, but for record literal arguments themselves. @stereotype441 thoughts on this?

[leafpetersen]

Another way of saying the above is that the proposal is not to do horizontal inference for records (because that doesn't really make sense), but instead to extend horizontal inference for functions to handle record literals "transparently". Which seems like a reasonable thing.

[stereotype441]

@leafpetersen said:

FWIW, this was a very common issue filed in the issue tracker after we rolled out null safety, so I suspect that even if the number of places it applied was small, the user visibility was relatively high. Also, are you sure you were capturing all places this applied? I currently see quite a lot of hits inside google for \.fold\< in null safe code, many of which I think are not now required because of your change.

Hmm, that's a good point. I just did the search you suggested and I see a lot of hits too. I'm not sure what led me to the conclusion that this logic had such a small effect. Sorry for the misinformation.

[stereotype441]

@leafpetersen

In other words, for horizontal inference, it would conceptually flatten out the entire tree of record literals inside function arguments to something like a linear list of leaf arguments/record fields, and then do horizontal inference across that flattened list.

That's fair, I think you could make sense of that. It's basically doing what I proposed for spreads of records, but for record literal arguments themselves. @stereotype441 thoughts on this?

I find Leaf's point about spreads pretty compelling. And I like the idea of flattening out records and spreads in the tree of arguments before doing horizontal inference. I believe it would be nontrivial, but tractable. So sure, consider this a "yes" vote from me 😃

[munificent]

Sounds good to me too. I think we'll be glad we did this and likely very glad when we get record spreading.

[eernstg]

I think I'll conclude that the answer is "yes", horizontal inference does apply for record literals. @stereotype441, do we need to mention this in an implementation issue somewhere?

[stereotype441]

I think I'll conclude that the answer is "yes", horizontal inference does apply for record literals. @stereotype441, do we need to mention this in an implementation issue somewhere?

Sure, I'll file an implementation issue.

FYI, I spoke to @leafpetersen about this yesterday, and although we both agreed that it would be good to do this, we also think it wouldn't be the end of the world if we ran out of time and had to postpone it to a future release. I'll prioritize it accordingly.

[stereotype441]

Implementation issue is here: dart-lang/sdk#50414

[leafpetersen]

we also think it wouldn't be the end of the world if we ran out of time and had to postpone it to a future release.

To expand on this further: I would be fairly surprised if the record literal (without a spread case) is at all common. The spread of a record literal feels like it might end up common, but ... we're not shipping spreads of record literals right now. So if this falls below the line, I think it would be fairly easy to add this as a language versioned change when/if we ship spreads of record literals.

[eernstg]

This confirms the status: We're aiming to get this done, but the timing is flexible. Thanks!

[Mike278]

FWIW, after I implemented horizontal inference, and updated Google internal sources to no longer use explicit types in the places where horizontal inference made them unnecessary, I was surprised how infrequently it actually made a difference in real-world code. Honestly, if I had it to do all over again, I probably wouldn't have even bothered implementing the feature in the first place, and would have spent my time on a feature with a greater user benefit.

@stereotype441 Anecdotally, it was massively appreciated for code bases which make heavy use of combineLatest* and similar APIs :)

[stereotype441]

@Mike278 good to know, thank you!