Skip to content

[CFE] Evaluate constants inside patterns #50629

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 Dec 5, 2022 · 2 comments
Closed

[CFE] Evaluate constants inside patterns #50629

alexmarkov opened this issue Dec 5, 2022 · 2 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@alexmarkov
Copy link
Contributor

Part of #49749.

main() {
  List value = const [42];
  if (value case const [42]) {
    print('OK');
  } else {
    print('FAIL');
  }
}

Kernel:

   core::List<dynamic> value = #C2;
    final core::List<dynamic> #t1 = value;
    final core::bool #t2 = true;
    if(#t1 =={core::List::==}{(core::Object) → core::bool} core::_GrowableList::_literal1<dynamic>(42)) {
      #t2 = false;
      {
        core::print("OK");
      }
    }
    if(#t2) {
      core::print("FAIL");
    }

The first const [42] was evaluated to a constant #C2, but the 2nd const [42] (in the pattern if (value case const [42])) was not evaluated. This results in runtime failures in certain tests (such as co19/LanguageFeatures/Patterns/constant_A03_t02, co19/LanguageFeatures/Patterns/constant_A03_t04 etc).

/cc @johnniwinther

@alexmarkov alexmarkov added the legacy-area-front-end Legacy: Use area-dart-model instead. label Dec 5, 2022
@johnniwinther
Copy link
Member

@stereotype441 This happens because the handleLiteralList call for the const [42] in value case const [42] doesn't have a constKeyword. The const keyword is instead provided to the handleConstantPattern method.

I've created https://dart-review.googlesource.com/c/sdk/+/273960 which instead includes the const keyword as part of the subexpression, thereby normalizing the handling of expression that can be prefixed by const.

@stereotype441
Copy link
Member

@bwilkerson what do you think of Johnni's AST changes in https://dart-review.googlesource.com/c/sdk/+/273960? They seem reasonable to me, but since you designed the analyzer's AST representation for patterns I want to get your input.

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