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

Adjust generator element type, cf. dart-lang/language#3148 #3218

Merged
merged 3 commits into from
Jul 26, 2023

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented Jul 17, 2023

In issue #3148, it is reported that there is a soundness issue with the computation of the element type of a generator function whose declared return type is FutureOr<Iterable<int>?>, and similar issues with other union types.

This PR changes the definition of this element type such that return types are simplified to Iterable<...> or Stream<...> before the element type is computed, if possible. Object and top types still yield the element type dynamic.

Moreover, this PR also adjusts the specification of the dynamic type of the returned iterable/stream accordingly.

NB: This PR introduces null-safety related material into the existing language specification. We have done this in other cases as well. In general, we're aiming to integrate all the null-safety related material into the language specification, piece by piece, so there is no violation of current practice in doing that with this PR as well.

@github-actions
Copy link

github-actions bot commented Jul 17, 2023

Visit the preview URL for this PR (updated for commit aa7f8c4):

https://dart-specification--pr3218-specify-generator-el-xal6crwg.web.app

(expires Tue, 01 Aug 2023 07:46:31 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 6941ecd630c4f067ff3d02708a45ae0f0a42b88a

@eernstg eernstg force-pushed the specify_generator_element_type_jul23 branch from cf28fdd to ee8256c Compare July 21, 2023 15:50
@eernstg
Copy link
Member Author

eernstg commented Jul 24, 2023

Friendly ping. Note that the current specification is unsound (cf. #3148 (comment)).

specification/dartLangSpec.tex Outdated Show resolved Hide resolved
specification/dartLangSpec.tex Outdated Show resolved Hide resolved
@leafpetersen
Copy link
Member

We will need a plan for making sure whatever specification we end up gets tested and implemented.

@leafpetersen
Copy link
Member

Also, how does this interact with https://github.com/dart-lang/language/pull/3151/files ?

@eernstg
Copy link
Member Author

eernstg commented Jul 25, 2023

[Edit: Corrected link]

We will need a plan

There is a co19 task in the pipeline, cf. dart-lang/co19#2142 dart-lang/co19#2139.

@sgrekhov
Copy link
Contributor

There is a co19 task in the pipeline, cf. dart-lang/co19#2142.

It's a dart-lang/co19#2139

@eernstg
Copy link
Member Author

eernstg commented Jul 25, 2023

how does this interact with https://github.com/dart-lang/language/pull/3151/files?

If we do not introduce new compile-time errors (for declarations like Iterable<int>? f() sync* {...}) then I believe this PR serves as a bug fix in the definition of 'the element type of a generator function'. In that case there wouldn't be any interactions, except that 'the element type of a generator function' is now less buggy.

@eernstg eernstg merged commit c13682a into main Jul 26, 2023
5 checks passed
@eernstg eernstg deleted the specify_generator_element_type_jul23 branch July 26, 2023 07:48
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 19, 2023
…c change

Calculation of element type for the value returned from
async/async*/sync* functions was recently adjusted
in the spec to address soundness issue
(dart-lang/language#3218).

This change adjusts Dart VM to conform to the new spec.

TEST=language/async/regression_54311_test
TEST=co19/Language/Functions/element_type_*

Fixes #54316
Issue #54311
Issue #54159

Change-Id: I4c51e7cba704d034350519375210bdb2086c5432
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342082
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Erik Ernst <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants