Should NNBD type inference infer required when necessary?

[leafpetersen]

Consider this API:

class A {
  void f(void Function({required int x})) {}
}

It seems natural to want to use this API as follows:

void test() {
  var a = new A();
  a.f(({x}) {});
}

As currently specified, however, this is an immediate error, since x is not required in the lambda. It seems useful to infer required here, since otherwise the code is guaranteed to be an error. Should we?

If so, in what circumstances do we do so? Consider:

1. a.f({x}) {}
2. a.f({required x}) {}
3. a.f({int x}) {}
4. a.f({int x = 42}) {}
5. a.f({int? x}) {}

Consider also the case where it would not be an error:

class A {
  void g(void Function({required int? x})) {}
}

Should we still infer required in a.g({x} {}) based on uniformity?

See also #630 .

cc @lrhn @eernstg @munificent

[lrhn]

There are arguments in either direction, but I feel like the arguments for are based on the syntax, not the semantics.

We already inherit covariant, so why not inherit required? But required is inconsistent syntax, the corresponding positional syntax is absence of [...] brackets. We would not suggest removing brackets to make foo([x]) override foo(int x), and I don't think we should do it for named parameters then.

I'd rather infer int? as type than required int as declaration during type inference. Even that is sort of arbitrary.

So I think we should just make it a compile-time error — you have to add either required, a nullable type or a default value to make the declaration valid, but we are not in a position to guess which one is the right one.

(Hmm, if I override foo(int x) with const num _x = 2;... foo([x = _x]) ..., will we infer num as type for x from the default value? If so, then consistency would allow us to infer int? for the case above from the implicit default value of null.)

[eernstg]

I argued in #630 (comment) that it is a net negative for developers if we infer a nullable parameter type T? where the context has T in order to avoid a non-type error. So I'd prefer inferring required and leaving the type unchanged.

@lrhn does have a point that it is a mere syntactic distinction that we are willing to infer required, but we won't use inference to "infer the brackets away" in the positional optional case. But I'm still willing to make that distinction, because adding required by inference will actually work as if we had added it by editing, but removing the brackets won't work (say, when the parameter in question is the second optional positional one, and we have no reason to make the first one required).

Inferring required for a formal parameter in a function literal is a much less disruptive action than inferring the type T? where the context has T: Code in the body of the function literal will never become an error due to that action, so the author of that function body can safely ignore it.

So I'd support any of the models that infer required, preferring that we only infer it in the cases where it is an error as long as required is not present (that is, minimally):

class A {
  void f(void Function({required int x})) {}
  void g(void Function({required int? x})) {}
}

void test() {
  var a = new A();
  a.f(({x}) {}); // OK, {required int x}.
  a.f(({required x}) {}); // OK, {required int x}.
  a.f(({int x}) {});  // OK, {required int x}.
  a.f(({int x = 42}) {}); // OK, unchanged.
  a.f(({int? x}) {}); // OK, unchanged.
  a.g(({x}) {}); // OK, {int? x}.
}

[leafpetersen]

ut required is inconsistent syntax, the corresponding positional syntax is absence of [...] brackets. We would not suggest removing brackets to make foo([x]) override foo(int x), and I don't think we should do it for named parameters then.

I don't find this logic very convincing. The syntax for optional positional parameters is not parallel to the syntax for required named parameters, and so arguing by parallelism between them doesn't work. Optional positional parameters are explicitly called out in a separate delineated section, whereas named parameters go into a single section, with required as a per variable modifier. So yes, I think if the syntax for required positional parameters was (required int x) and the absence of the required modifier meant is was optional positional then we would also be talking about doing this for positional parameters. But it's not, so we don't.

More to the point, there is a huge difference between ignoring something that the user explicitly wrote (treating [x] as x) and adding something that the user didn't write, but which the compiler can reasonably figure out is intended. ({x}) when ({required x}) is required. The latter is inference, the former is weird.

So I think we should just make it a compile-time error — you have to add either required, a nullable type or a default value to make the declaration valid, but we are not in a position to guess which one is the right one.

Why not? There is incredibly strong evidence in the program as to what the right one is. The context type says ({required int x}), filling in the missing required will almost always do the right thing, and not filling in the missing required means that the obvious, short, code has to become pointlessly verbose. If it weren't for the fact that Dart lets you introspect on runtime types, we could say something stronger: filling in the missing required always does the right thing.

The argument that we can't guess the right thing to do here applies exactly as well to type inference. How do we know that given void Function(int x) f = (x) {} the user doesn't intend x to have type Object? They could follow this up with assert(f is void Function(Object)) so surely inferring int as the type of x in the lambda is a mistake? Well, no - in practice, when assigning a lambda to a typed location, you pretty much always expect it to have the type you're assigning it to. There's an obvious correct default to infer, and if the user wants something different, they can make it explicit.

[rrousselGit]

So I think we should just make it a compile-time error — you have to add either required, a nullable type or a default value to make the declaration valid, but we are not in a position to guess which one is the right one.

What makes that adding the required implicitly may be a mistake? These cases are not immediately obvious to me.

Are these cases something that people write often, or just rare edge cases?

[lrhn]

If the author writes nothing in the subclass, they inherit the superclass type. I'm fine with also inheriting other annotations in that case.

So, the superclass has int foo({required int x});. The subclass declares foo({x}). It then inherits required int.
That allows the briefest of code to be correctly inferred. If you start writing some of the declaration, it's less clear that we should be inserting the rest.

However, if the subclass writes foo({int x}), I'd say they have left "inherit" mode. In that case, I don't want to infer required. We can, but I don't want to, because it's not the only option available. All we can see is that they wrote something which is wrong, but any of required, nullable type or default value are solutions, and I don't want inference to guess.
(If the subclass writes foo({required x}), I guess we will inherit the type. That is an argument against my position being consistent.).

I don't think there is incredibly strong evidence what the right thing is. Maybe I'm just being obtuse, that's a definite possibility.

The one thing that would favor required over a nullable type or default value is that people rarely change the signature when overriding, and being required is the strongest restriction available. Choosing that won't accidentally allow some code that wasn't intended. So, if we are going to choose any option, that's the safest option to choose. (The same reason we choose the most specific type when inferring parameter types).

Would inferring required also apply to:

foo(int bar({required int x}) { ... }
foo(({x]) => x); //?
foo(({int x}) => x); //?

?

Again, I'd be OK with doing it for {x}, but less OK for {int x}. The user wrote something, they didn't need to, so something non-obvious might be happening. Let's not try to guess what.

As a counter-opinion, it's not a good user experience when providing more correct information gives you worse inference.
If writing nothing gives you required int, writing any of required or int should still give you the other one, otherwise our inference isn't monotonic in the available information.

I think that principle actually wins for me, so ... let's inherit/infer required in all the places where we would infer required int with nothing but x written.

[munificent]

I'm sympathetic to the general desire. I think if we don't do this for lambdas, users will be annoyed (though lambdas with named parameters are relatively rare). However, I would feel weird about adding an inference feature for lambdas that does not have matching behavior in override inference. If we supported this for overrides, how would it compose?

Given:

class A {
  foo({required i}) {}
  bar({required i}) {}
}

mixin M {
  foo({int i}) {}
  bar({i}) {}
}

class B extends A with M {
  foo({i}) {}
  bar({i}) {}
}

What are the inferred types of foo() and bar() in B? In other words, how do we distinguish between "This method explicitly claims the parameter is not required" from "This method makes no claims regarding required-ness at all?"

I'm less worried about the idea that inferred required named parameters doesn't match now optional positional parameters works. The syntax for those is quite different and encourages a different mental model. With named parameters, the parameter is there and required-ness is an attribute of it. With positional parameters, there are two independent sets of parameters, the required and optional ones. Optional-ness is part of the parameter's identity.

[leafpetersen]

If the author writes nothing in the subclass, they inherit the superclass type. I'm fine with also inheriting other annotations in that case.

@lrhn I don't understand where you got to inheritance - nothing in this issue is about inheritance based inference. We could consider doing inheritance based inference for this, but that isn't what I filed this issue about.

Would inferring required also apply to:

foo(int bar({required int x}) { ... }
foo(({x]) => x); //?
foo(({int x}) => x); //?

?

That is what this issue is about.

[leafpetersen]

However, I would feel weird about adding an inference feature for lambdas that does not have matching behavior in override inference.

@munificent We already do pretty different things here. I guess not doubling down is an argument, but they're different enough that it hadn't even occurred to me to consider proposing this for override based inference. In general, I don't think override inference carries it's weight - very few users know it exists, and I don't see it very widely used. I don't object to adding this there, but I think very few users expect to be able to write a method with no types and get typed code, whereas they expect to be able to write lambdas with no types and get typed code.

What are the inferred types of foo() and bar() in B? In other words, how do we distinguish between "This method explicitly claims the parameter is not required" from "This method makes no claims regarding required-ness at all?"

@munificent In your example, M is erroneous no matter what. Did you mean to write something else? Maybe make M abstract? In any case, I think the mixin is irrelevant here - we don't do any override inference for the mixin methods at mixin applications. So the following is a valid program because M.foo has argument type dynamic, which is never subject to override inference, and override inference for B.foo is done relative to M.foo.

class A {
  foo({int i = 3}) {}
}

mixin M {
  foo({i}) {}
}

class B extends A with M {
  foo({i}) {
    i.arglebargle;
  }
}

[lrhn]

I can see that I lost track somewhere throughout the thread. It is indeed about function literals.

I think we should be doing the same thing here as wrt. inheritance, so whatever we decide in either case should apply in both.

I think (now) that we should at least infer required when the author does not write it, the context type has required, the code is invalid without the required and would be valid with it.

So for context type Function({required int x}), the following will be required:

({x}) {}  ↦ ({required int x}){}
({required x}) {} ↦ ({required int x}){}
({int x}) {} ↦ ({required int x}){}

The cases where it's not necessary, the required will not be added. That would leave no way to make the parameter actually not required. Also, it's not required.

({int? x}) {} 
({int x = 42}) {}

If they write a different type which is still non-nullable, I'm not sure whether to keep the required. We probably should because it's easier to explain.

({num x}) {} ↦ ({required num x}){}

If the type of the context type parameter doesn't require the parameter to be required, we do not insert it. With context type Function({required int? x}), the following will not infer required:

({x}) ()
({int? x}) ()
({num? x}) ()

It's not necessary, so if they actually want the parameter to be non-required, they need a way to write that.
(It's also unlikely to be important. A literal function in a context is most likely only going to be used at the context type. Whether a parameter is actually required or not is not important if the function is being used at a type where the parameter is required.)

So, only insert required when:

• The context type has required.
• The function parameter declaration is invalid.
• Inserting required would make it valid.

The only question left to answer is why is this different from default values?

If we have

void foo({int x = 42}) => ...;
...
   foo(({x}) {});

then the function literal is not valid after inferring int as type. We do not fix that (#630).
Fixes could be making the type nullable or inserting a default value (making the parameter required is not an option). We do neither.
Why do we treat required differently? Is it just because it's easy?

[leafpetersen]

The only question left to answer is why is this different from default values?

Because default values aren't part of the types, and we do inference from the types.

If we have

void foo({int x = 42}) => ...;
...
   foo(({x}) {});

I don't know what this program is supposed to be? It's passing a function where an integer is expected.

Fixes could be making the type nullable or inserting a default value (making the parameter required is not an option). We do neither.
Why do we treat required differently? Is it just because it's easy?

In general, we can't invent a default value, and we'd have to since it's part of the type.

For the "making the type nullable" version, that was at least partially the proposal in the issue that this issue was split off from. It is something we could do. I think the argument for doing so is less compelling to me though. Some examples where we could do this:

• void Function(int?) is expected, the user provided (int x) {...}
  • If the user wrote a type, then modifying that type is a little weird.
• void Function(int) is expected, the user provided ([x]) {...}
  • The user is doing something weird, and probably not intended here. Why not just use `(x) {...}?

All of this said, @munificent makes the fair point that he believes that closures with named parameters are very rare. His argument is that this is rare enough to make this really not worth spending the effort, time, and cognitive budget on. That seems like a fair point, and he and @lrhn are in a better position to judge this than I am. So maybe not worth doing?

[eernstg]

I'd support inferring required (but not changing the parameter type to nullable) whenever this eliminates a compile-time error. This also makes the change non-breaking.

So we could say "This may be worthwhile, but it is not top priority, and we can do it later".

[munificent]

@munificent makes the fair point that he believes that closures with named parameters are very rare.

I got data! I cobbled together a little script and ran it on the Flutter repo, Dart SDK repo, and the 2,000 most recent packages on pub, as of today. That's 14,399,669 lines of Dart code in 46,082 files. Here is a histogram of every parameter signature in every lambda:

--- Signatures (245309 total) ---
 117277 ( 47.808%): ()                       ************************************************
 105895 ( 43.168%): (_)                      ********************************************
  19897 (  8.111%): (_,_)                    *********
   1245 (  0.508%): (_,_,_)                  *
    278 (  0.113%): (_,_,_,_)                *
    139 (  0.057%): ([_])                    *
    101 (  0.041%): ({_})                    *
     95 (  0.039%): (_,[_])                  *
     52 (  0.021%): (_,_,_,_,_)              *
     48 (  0.020%): (_,{_})                  *
     35 (  0.014%): ([_,_,_,_,_,_,_,_,_,_])  *
     23 (  0.009%): ({_,_})                  *
     22 (  0.009%): ({_,_,_,_})              *
     21 (  0.009%): (_,{_,_,_})              *
     20 (  0.008%): (_,_,[_,_])              *
     19 (  0.008%): (_,{_,_})                *
     18 (  0.007%): (_,_,{_,_})              *
     17 (  0.007%): (_,_,_,_,_,_)            *
     16 (  0.007%): (_,_,[_])                *
     14 (  0.006%): ({_,_,_,_,_})            *
      9 (  0.004%): (_,_,_,_,_,_,_)          *
      9 (  0.004%): ({_,_,_})                *
      9 (  0.004%): (_,_,_,_,_,_,_,_)        *
      8 (  0.003%): (_,{_,_,_,_,_,_})        *
      6 (  0.002%): (_,_,{_})                *
      5 (  0.002%): ([_,_])                  *
      4 (  0.002%): ([_,_,_,_,_])            *
      4 (  0.002%): (_,[_,_,_,_,_,_])        *
      3 (  0.001%): ([_,_,_,_])              *
      3 (  0.001%): ({_,_,_,_,_,_})          *
      2 (  0.001%): ([_,_,_])                *
      2 (  0.001%): ({_,_,_,_,_,_,_,_,_})    *
      2 (  0.001%): (_,_,{_,_,_})            *
      2 (  0.001%): (_,_,_,[_])              *
      2 (  0.001%): (_,[_,_])                *
      1 (  0.000%): ({_,_,_,_,_,_,_,_,_,_})  *
      1 (  0.000%): (_,{_,_,_,_,_,_,_,_})    *
      1 (  0.000%): (_,_,_,[_,_])            *
      1 (  0.000%): (_,_,{_,_,_,_})          *
      1 (  0.000%): (_,_,_,{_})              *
      1 (  0.000%): (_,_,_,_,_,{_,_})        *
      1 (  0.000%): (_,_,[_,_,_,_])          *

This is a histogram which counts number of occurrences of each type. For example, this means that there are 117,277 lambdas whose parameter signature is () (no params), which is 47.808% of all lambdas.

Bucketing by what kinds of parameters appear:

--- Types (245309 total) ---
 127402 ( 51.935%): only positional          ****************************************************
 117277 ( 47.808%): no params                ************************************************
    188 (  0.077%): only optional            *
    175 (  0.071%): only named               *
    141 (  0.057%): positional and optional  *
    126 (  0.051%): positional and named     *

So of the quarter million lambdas, only 301 (about 0.1%, or one in a thousand) have any named parameters.

[eernstg]

That is cool!
Looking at these results, my impression remains that this feature may be nice, but it is not urgently needed.

[leafpetersen]

@munificent Thanks! That's tremendously useful! I'm pretty inclined not to spend effort on this feature then, sorry for the noise. @lrhn does that seem reasonable to you as well?

[rrousselGit]

I don't understand why closure not using named parameters influence the decision, unless I am misunderstanding something? The inference of required would matter for constructors and methods mostly, not closures. I'm probably missing something

…

On Fri, May 15, 2020, 01:49 Leaf Petersen ***@***.***> wrote: @munificent <https://github.com/munificent> Thanks! That's tremendously useful! I'm pretty inclined not to spend effort on this feature then, sorry for the noise. @lrhn <https://github.com/lrhn> does that seem reasonable to you as well? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#938 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEZ3I3M3225HX6DSZUDHKSDRRSGQ3ANCNFSM4MPUXPYQ> .

[leafpetersen]

I don't understand why closure not using named parameters influence the decision, unless I am misunderstanding something?

This issue was primarily (from my standpoint) about closures. That is by far the most common place for parameter type inference to be done, and by far the place where brevity seems most valued by users (see for example Kotlin where even eliding the parameter is a feature). It's also a place where inference only matters to the local code. That is, given f((x) => e), the inferred type of x only matters in the body of e. So the effects of inference are very local, and very visible to the person writing the code.

The inference of required would matter for constructors and methods mostly, not closures.

There is no inference for constructors. For methods, the only relevant inference would be override based inference: that is, given a super class method R m({required T x}) and an overriding sub class method R m({x}), we could infer both the required and the T.

We could still do this (or really, do both, since if we do one it feels like we should do the other), and it is true that named method parameters are fairly common.

On the other hand, my sense is that override inference is not very widely used, and frankly it's not clear to me that it's a feature we should encourage users to lean on. In general, my experience with type inference from other languages is that as a general rule you don't want your API surface to be inferred. It's good documentation and good error checking to have API be explicitly written out.

That's my take anyway.

[rrousselGit]

I'm not talking about override inference, but new constructors and new functions. I don't think the inference of "required" uses the same rules. We are not inferring the parameter type, but the fact that it is required. The parameter type is unchanged Inferring: ```dart void function({required int x}) {} ``` When the user writes ```dart void function({int x}) {} ``` Is harmless. There is only a single possibility where this code can work, as this code otherwise does not compile. There is no unknown behavior or scenarios where that's not what we want. In 100% of the situations, writing the above code can only mean that the parameter is required. So having to write it is redundant and decrease readability.

[lrhn]

@rrousselGit

There is only a single possibility where this code can work, as this code
otherwise does not compile.

I've argued above that there is more than one possibility. Adding required, ? or a default value will all work.
Inventing a default value is tricky (but inheriting it should be safe).
Adding ? is changing the signature, and it might make the body invalid. It might not, and then it's probably (but not certainly) the right thing.
Adding required is also changing the signature, but only invalidates calls, which probably don't exist yet when the code isn't compiling yet.

And for an abstract function, you don't need to add required, so the difference in behavior between abstract and concrete functions is probably going to be confusing.

I'm (now) OK with copying whatever requirement is provided externally by another function signature (required parameter in context type for literals, required or default value in super-class declaration for overrides), but not inventing something on the spot to make invalid code valid.

[rrousselGit]

And for an abstract function, you don't need to add required, so the

difference in behavior between abstract and concrete functions is probably going to be confusing. I didn't think of that one. That's fair then. Le ven. 15 mai 2020 à 09:29, Lasse R.H. Nielsen <notifications@github.com> a écrit :

…

@rrousselGit <https://github.com/rrousselGit> There is only a single possibility where this code can work, as this code otherwise does not compile. I've argued above that there is more than one possibility. Adding required, ? or a default value will all work. Inventing a default value is tricky (but inheriting it should be safe). Adding ? is changing the signature, and it might make the body invalid. It might not, and then it's probably (but not certainly) the right thing. Adding required is also changing the signature, but only invalidates calls, which probably don't exist yet when the code isn't compiling yet. And for an abstract function, you don't need to add required, so the difference in behavior between abstract and concrete functions is probably going to be confusing. I'm (now) OK with copying whatever requirement is provided externally by another function signature (required parameter in context type for literals, required or default value in super-class declaration for overrides), but not inventing something on the spot to make invalid code valid. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#938 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEZ3I3MK3F65RVCBJNODXQ3RRT4PVANCNFSM4MPUXPYQ> .

[munificent]

Inferring:

void function({required int x}) {}

When the user writes

void function({int x}) {}

Is harmless.

You are asking about a different kind of inference here. In your example, you are asking about inferring required from the parameter's own static type. This issue is about inferring required from the downwards inference context.

Your request was considered in detail here: #156 (and in particular this comment), and #878. We decided it wasn't worth the potential confusion, in part because of the issue with abstract methods Lasse mentions here.

[leafpetersen]

This is not planned.