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

[CP] Implement error about awaiting a type that is incompatible with await #55095

Closed
stereotype441 opened this issue Mar 4, 2024 · 3 comments
Assignees
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable

Comments

@stereotype441
Copy link
Member

Commit(s) to merge

bf17088, 1672177, a5bbbe8, adee0a2, 690d49c

Target

Stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/355542

Issue Description

The new extension types feature contains a rule preventing an extension type from being used as the operand of an await, unless the extension type implements Future. However, the logic that was released in Dart 3.3.0 has a loophole in that it allows some types based on an extension type to be used as the operand of an await, such as:

  • A nullable extension type where the extension type doesn't implement future.
  • A promoted type variable, where the promoted type is incompatible with await.
  • A type variable with a bound that is incompatible with await.

The loophole was fixed in the specification on Jan 17 (dart-lang/language#3560) and Jan 31 (dart-lang/language#3598). However, the implementation of this fix didn't land until after the release cutoff for Dart 3.3. This cherry-pick closes the loophole for the stable release.

What is the fix

Why cherry-pick

Since the extension types feature is so new, it's highly unlikely that it's seen enough widespread usage for anyone to have run into the loophole described above. It's far better to cherry-pick a fix to the loophole now, than to defer the fix until Dart 3.4.0 (at which point, use of the feature will probably be much more widespread).

Risk

low

Issue link(s)

#54647

Extra Info

No response

@stereotype441 stereotype441 added the cherry-pick-review Issue that need cherry pick triage to approve label Mar 4, 2024
@itsjustkevin
Copy link
Contributor

@johnniwinther and @mraleph can you take a look at this cherry-pick request.

@mraleph
Copy link
Member

mraleph commented Mar 5, 2024

LGTM based on the following:

  • Change localized to a newly released feature;
  • Good test overage;
  • Desire to prevent subsequent breaking change in the next release;

@johnniwinther
Copy link
Member

LGTM

@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-approved Label for approved cherrypick request labels Mar 5, 2024
copybara-service bot pushed a commit that referenced this issue Mar 5, 2024
…ith await

This is a cherry-pick of the following 5 commits from the main branch:

- Add test about types being incompatible with await
  (https://dart-review.googlesource.com/c/sdk/+/351143)

- [cfe] Report errors on awaiting types incompatible with await
  (https://dart-review.googlesource.com/c/sdk/+/348640)

- [cfe] Implement the update of "incompatible with await"
  (https://dart-review.googlesource.com/c/sdk/+/350986)

- Extension type. Implement isIncompatibleWithAwait() predicate,
  report AWAIT_OF_INCOMPATIBLE_TYPE.
  (https://dart-review.googlesource.com/c/sdk/+/350986)

- Extension type. Issue 54648. Fix 'incompatible with await'
  predicate. (https://dart-review.googlesource.com/c/sdk/+/355500)

Collectively, these 5 commits implement the new "incompatible with
await" error that was specified in
dart-lang/language#3560 and then refined in
dart-lang/language#3598.

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/348640
Cherry-pick-request: #55095
Change-Id: I84a79ffccce89c4d91c99a09ab7f5107a96e9844
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355542
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable
Projects
None yet
Development

No branches or pull requests

8 participants