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

Wrong run-time type of the value returned by generator function #53051

Closed
sgrekhov opened this issue Jul 27, 2023 · 4 comments
Closed

Wrong run-time type of the value returned by generator function #53051

sgrekhov opened this issue Jul 27, 2023 · 4 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@sgrekhov
Copy link
Contributor

sgrekhov commented Jul 27, 2023

Why run-time type of the variable from the below is Future?

import "dart:async";

FutureOr<Stream<int>?> f1() async* {
  yield 1;
  yield 2;
  yield 3;
}

main() async {
  Stream<int>? res1 = await f1();
  List<int>? it1 = await res1?.toList();
  print(it1 is List<int>?); // Analyzer: Unnecessary type check; the result is always 'true'.
                            // Prints `false`
  print(it1.runtimeType);   // Future<List<dynamic>>
}

Ok, the fact that type of it1 is List<dynamic> instead of List<int> is a known issue described here. But why it is Future?

Note that if to remove nullability issue connected with the Future disappers

import "dart:async";

FutureOr<Stream<int>> f1() async* {
  yield 1;
  yield 2;
  yield 3;
}

main() async {
  Stream<int> res1 = await f1();
  List<int> it1 = await res1.toList();
  print(it1 is List<int>); // Analyzer: Unnecessary type check; the result is always 'true'.
                            // Prints `false`
  print(it1.runtimeType);   // List<dynamic>
}

Tested on Dart SDK version: 3.1.0-333.0.dev (dev) (Thu Jul 20 13:04:10 2023 -0700) on "windows_x64"

cc @eernstg @lrhn

@eernstg
Copy link
Member

eernstg commented Jul 27, 2023

This is the bug which was reported here, which gave rise to dart-lang/language#3218.

With the old rules, f1 would have element type dynamic, so it returns a Stream<dynamic>, but the static type is FutureOr<Stream<int>?> (so this is already unsound). With the current rules it would have element type int, so it should return a Stream<int>, but that hasn't been implemented yet.

Next, we get the toList() and that turns out to be a Future<List<dynamic>>, with static type Future<List<int>?>. So the static type of the await is List<int>?, but the dynamic type is Future<List<dynamic>> (with the correct semantics of await) or List<dynamic> (if the runtime doesn't implement the required dynamic type check, cf. #50601). With DartPad we get the latter, so it prints JsArray<dynamic>.

Presumably you ran the experiment using dart on the command line (or some other non-dart2js runtime), so you got Future<List<dynamic>>.

So it's all working or failing as expected, considering the known issues.

The second example can be explained along the same lines, but in this case there is no doubt: We get a List<dynamic> at the end, because the type check is skipped (we don't check whether it's actually a Future<List<int>>, so we don't discover that it is actually not, it is a Future<List<dynamic>>, all because the program is unsound and it is only OK to skip that type test when the program is sound).

@sgrekhov
Copy link
Contributor Author

I tested command-line dart on Windows and Linux (freshly built) with no any experimental flags. Both give Future<List<dynamic>> as it1.runtimeType in the first example.

To clarify. We have an known issue here the List type is dynamic instead of int (if we don't have a SDK issue for that I'll file the new one later).

But if the fact, that variable declared as List<dynamic>? (let's have List<dynamic>? instead of List<int>? to separate the issues) has a run-time type Future<List<dynamic>> is an issue or not?

@eernstg
Copy link
Member

eernstg commented Jul 27, 2023

Both give Future<List<dynamic>> as it1.runtimeType in the first example.

Yes, that's because they are running the VM, and the VM has a fix for #49396, but dart2js does not (and that's the reason why #50601 is still open, at least for a couple of days yet ;-).

So the VM behavior is correct here, and the dart2js behavior is not. I think DDC has the fix as well.

But none of the tools have a fix (yet) for the generator element type bug (that was a spec bug), which is the reason why we get a Stream<dynamic> in the first place. That's also the reason why we get a Future<List<dynamic>?> from toList() later on, and then a List<dynamic>.

So we have one issue which is #49396, which is just about to be completely fixed. Next, we have one fault which is about the element type of a generator, and there is no github issue for that, yet (and the spec change just landed). I just created #53052 in order to address the updates needed now that the spec bug has been fixed.

@sgrekhov sgrekhov added the closed-as-intended Closed as the reported issue is expected behavior label Jul 27, 2023
@sgrekhov
Copy link
Contributor Author

Ok, thak you for clarification. Closing this issue then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

2 participants