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][stable channel] Improve messaging on invalid use of >>> #47187

Closed
johnniwinther opened this issue Sep 10, 2021 · 2 comments
Closed

[CP][stable channel] Improve messaging on invalid use of >>> #47187

johnniwinther opened this issue Sep 10, 2021 · 2 comments
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. cherry-pick-approved Label for approved cherrypick request merge-to-stable

Comments

@johnniwinther
Copy link
Member

commit(s) to merge:
01212e3
72de10d
1231e3d

merge instructions: https://dart-review.googlesource.com/c/sdk/+/213043

What is the issue: Using the >>> operator in a <2.14 library results in confusing parser errors because >>> is parsed as >> followed by a >, instead of mentioning that >>> needs the triple-shift language feature. Running dart on this program

// @dart=2.13
class Class {
  operator >>>(o) => null;
}
method(int i, int j) {
  i >>> j;
  i >>>= j;  
}

results in these confusing messages

test.dart:3:12: Error: A method declaration needs an explicit list of parameters.
Try adding a parameter list to the method declaration.
  operator >>>(o) => null;
           ^^
test.dart:3:14: Error: Expected '{' before this.
  operator >>>(o) => null;
             ^
test.dart:3:12: Error: Operator '>>' should have exactly one parameter.
  operator >>>(o) => null;
           ^^
test.dart:3:14: Error: Operator declarations must be preceded by the keyword 'operator'.
Try adding the keyword 'operator'.
  operator >>>(o) => null;
             ^
test.dart:6:7: Error: Expected an identifier, but got '>'.
Try inserting an identifier before '>'.
  i >>> j;
      ^
test.dart:7:7: Error: Expected an identifier, but got '>='.
Try inserting an identifier before '>='.
  i >>>= j;
      ^^

What is the fix: The fix is to special case >> immediately followed by > in the parser. This is done just after parsing operator > and just before parsing >> followed by > as a binary expression. At these points the normal message for an unavailable language feature is reported, instead the parser error it would normally have led to.

This means that after the fix, the output for the program above is:

test.dart:4:12: Error: This requires the 'triple-shift' language feature to be enabled.
Try updating your pubspec.yaml to set the minimum SDK constraint to 2.14 or higher, and running 'pub get'.
  operator >>>(o) => null;
           ^^^
test.dart:8:5: Error: This requires the 'triple-shift' language feature to be enabled.
Try updating your pubspec.yaml to set the minimum SDK constraint to 2.14 or higher, and running 'pub get'.
  i >>> j;
    ^^^
test.dart:9:5: Error: This requires the 'triple-shift' language feature to be enabled.
Try updating your pubspec.yaml to set the minimum SDK constraint to 2.14 or higher, and running 'pub get'.
  i >>>= j;
    ^^^^

Why cherrypick: Since the triple-shift feature is launched with the upcoming stable release, the messaging should clearly recognize failed attempts to use the features, and not indicate that the feature is unknown.

Risk: Low-medium risk. The parser change is added at points where parser errors are inevitable. The fix has been verified to work without cascading error messages.

Link to original issue(s): #46886

/cc @kevmoo @mit-mit @whesse @athomas @vsmenon @devoncarew @lrhn @jensjoha

@johnniwinther johnniwinther added merge-to-stable cherry-pick-review Issue that need cherry pick triage to approve labels Sep 10, 2021
@lrhn lrhn added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Sep 10, 2021
@devoncarew
Copy link
Member

Approved; cc @athomas.

@devoncarew devoncarew added cherry-pick-approved Label for approved cherrypick request and removed cherry-pick-review Issue that need cherry pick triage to approve labels Sep 14, 2021
@athomas
Copy link
Member

athomas commented Sep 15, 2021

Merged to stable in 3300f32 (2.14.2).

@athomas athomas closed this as completed Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. cherry-pick-approved Label for approved cherrypick request merge-to-stable
Projects
None yet
Development

No branches or pull requests

4 participants