Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent inferred return type of function literal with specialized body #3715

Open
eernstg opened this issue Apr 19, 2024 · 1 comment
Open
Labels
inference question Further information is requested type-inference Type inference, issues or improvements

Comments

@eernstg
Copy link
Member

eernstg commented Apr 19, 2024

Consider the following program:

// Example 1.

void f(void Function() g) => print(g.runtimeType);

void main() {
  f(() {}..expectStaticType<Exactly<void Function()>>());
  f(() sync* {}..expectStaticType<Exactly<void Function()>>());
  f(() async {}..expectStaticType<Exactly<void Function()>>());
  f(() async* {}..expectStaticType<Exactly<void Function()>>());
}

typedef Exactly<X> = X Function(X);

extension<X> on X {
  X expectStaticType<Y extends Exactly<X>>() => this;
}

This program has an implied expectation that the inferred return type of the function literal is void in all four cases (if it isn't void or some other top type then we'll have a compile-time error).

The reason why the function literal return type should be void is the following rule from this section:

if R is void, or the function literal is marked async and R is FutureOr<void>, let S be void

In the context, R is the 'imposed return type schema' (which is obtained from the context type of the function literal) and S is the inferred return type of the function literal. In the example above R is void, and we're computing the value of S. The rule cited above says that S should be void. This result is applicable to all kinds of function literals (and we don't have a context type whose return type is FutureOr<void> and hence we can ignore the part between the commas).

The motivation for this rule is that this will allow function literals to have the same kind of void related sanity checks that a regular function gets. For example:

// Example 2. 

void main() {
  void Function() f1 = () {
    return 3; // Compile-time error.
  };
  return 3; // Compile-time error.
}

In this example we get a compile-time error indicating that we cannot return 3 from a function whose return type is void, at both occurrences of return 3;.

In general, this compile-time error is not required for soundness reasons, it's an opinionated rule that gives developers a heads-up when they write code that seems to imply that a given non-void value (here: 3) is being returned to someone, but the value will actually (almost certainly) be ignored, because the return type is void. That is considered to be a likely bug, hence the error.

For a sync* or async* function it is an error to declare the return type to be void. This implies that when the inferred return type is void, a compile-time error is reported, and the developer will then know that it is a likely bug to use a sync* or async* function in that manner, just like they'd encounter a compile-time error if they were to declare a function like void f() sync* {...}.

For an async function there may or may not be a lint message warning against using the return type void in a declaration of an async function. Again, we get the same kind of feedback for the regular function with return type void as we do for the function literal whose context type has return type void.

The conclusion so far is that in example 1, the function literal should have the type void Function() in all four cases.

However, this is not the behavior that we can observe. Here are the adjusted versions of example 1 that are accepted without compile-time errors by the analyzer respectively the CFE:

// Example 1, adjusted to fit the analyzer.

void main() {
  f(() {}..expectStaticType<Exactly<void Function()>>());
  f(() sync* {}..expectStaticType<Exactly<Iterable<void> Function()>>());
  f(() async {}..expectStaticType<Exactly<Future<void> Function()>>());
  f(() async* {}..expectStaticType<Exactly<Stream<void> Function()>>());
}
// Example 1, adjusted to fit the CFE.

void main() {
  f(() {}..expectStaticType<Exactly<void Function()>>());
  f(() sync* {}..expectStaticType<Exactly<Iterable<Null> Function()>>());
  f(() async {}..expectStaticType<Exactly<Future<void> Function()>>());
  f(() async* {}..expectStaticType<Exactly<Stream<Null> Function()>>());
}

As we can see, the tools do not use the rule mentioned above to make the inferred return type void, they use the kind of return type which is associated with the given kind of function body. However, they do not agree on the type argument.

It is in principle a breaking change to change the inferred return type to void. However, it is likely to bring out locations in code where something unintended is taking place, so it's arguably "good breakage".

So, @dart-lang/language-team, WDYT? Should we change the specification to say that the sync*, async, async* function literals should have a return type of the form Iterable<...>, Future<...>, respectively Stream<...>? In that case, what's the actual type argument? Alternatively, should we ask the tool teams to infer void and report a compile-time error?

@eernstg eernstg added question Further information is requested inference type-inference Type inference, issues or improvements labels Apr 19, 2024
@lrhn
Copy link
Member

lrhn commented Apr 19, 2024

The current specification does have the advantage that you get errors for the sync* and async* functions in void-function contexts. That's what you'd want.
It's also potentially breaking, but if anyone gets hit by that, they should feel lucky, because their function isn't working.

We do get the same warnings for:

Future<void> f() async { return 42; }
void f() async { return 42; }

and no warnings for yield 42; whether in a void or Stream<void>/Iterable<void> generator, so for other warnings it doesn't matter whether we do as specified or as the analyzer does.

If we do want to change, I'd go with "what the analyzer does". I do want the element type to be void.

(I'd also want the analyzer to warn about any sync* or async* function being passed in where a void function is expected, because the function call will never do anything. An actual call will return an Iterable/Stream which is then discarded, that's what being assigned to void means, so no real computation will ever happen. @bwilkerson)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inference question Further information is requested type-inference Type inference, issues or improvements
Projects
None yet
Development

No branches or pull requests

2 participants