Skip to content

[Records] CFE doesn't correctly handle ambiguity with on clauses #49980

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

Closed
alexmarkov opened this issue Sep 16, 2022 · 4 comments
Closed

[Records] CFE doesn't correctly handle ambiguity with on clauses #49980

alexmarkov opened this issue Sep 16, 2022 · 4 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@alexmarkov
Copy link
Contributor

Part of #49713.

According to the spec https://github.com/dart-lang/language/blob/master/accepted/future-releases/records/records-feature-specification.md#ambiguity-with-on-clauses try .. on .. on (a, b) {} should be parsed as try with 2 on clauses.
However, front-end incorrectly parses it as try .. on and a local function.

This causes co19/LanguageFeatures/Records/on_clauses_A01_t01 test failure with RuntimeError.

Kernel AST of this test:

    try {
      throw (42, "Lily was here");
    }
    on core::String catch(no-exception-var) {
      exp::Expect::fail("Unexpected String exception");
    }
    function on(core::int i, core::String n) → Null {
      caught = true;
    }

@johnniwinther

@alexmarkov alexmarkov added the legacy-area-front-end Legacy: Use area-dart-model instead. label Sep 16, 2022
@jensjoha
Copy link
Contributor

I know. But considering record types are probably about to get some different syntax (dart-lang/language#2469) it feels like a waste of time to change this.

@scheglov
Copy link
Contributor

dart-lang/language#2599 updated the specification, so this is still an issue.

Specifically, this:

main() {
  try {
    print(0);
  } on String {
  } on ([int? i, String? n]) {
//      ^
// [analyzer] unspecified
// [cfe] unspecified
  }
  on();
//^^
// [analyzer] unspecified
// [cfe] unspecified
}

is parsed as:
image

@scheglov scheglov reopened this Nov 16, 2022
@scheglov
Copy link
Contributor

scheglov commented Jan 3, 2023

@jensjoha I think there is still work to do here.

@jensjoha
Copy link
Contributor

jensjoha commented Jan 4, 2023

It's in flight (https://dart-review.googlesource.com/c/sdk/+/275922) delayed by the holiday break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.
Projects
None yet
Development

No branches or pull requests

3 participants