Skip to content

Backporting .fromEnvironment updates: constant evaluation of bool.hasEnvironment #41051

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
eernstg opened this issue Mar 16, 2020 · 0 comments
Closed
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release

Comments

@eernstg
Copy link
Member

eernstg commented Mar 16, 2020

Cf. breaking change #40678, https://dart-review.googlesource.com/c/sdk/+/139286 introduced bool.hasEnvironment into the sdk/sdk_nnbd.

The next step in the CFE would be to support constant evaluation of expressions like const bool.hasEnvironment("name").

If the CFE uses hardwired default values for the defaultValue parameters of fromEnvironment constructors then it would probably be needed to switch to obtain those default values from the declarations, such that it would work with both the sdk and the sdk_nnbd defaults without special casing.

With this implementation, the new tests 'language{,_2}/bool/has_environment_test.dart' should pass. However, the vm does not (and never did) enforce the constraint that fromEnvironment constructors (and now also the hasEnvironment constructor) must throw if invoked at run time. So the tests 'language{,_2}/bool/has_environment_not_new_test.dart' will only succeed if the CFE detects that a particular invocation of these constructors will take place at run time (and then generates code to throw at run time), and that would be a new behavior. I mentioned this to @mkustermann, and he agreed that it would be acceptable to let these tests fail, that is, to continue to omit support for this particular error in the vm.

@johnniwinther, can you help finding a way to fit this into the cfe team work plan?

@eernstg eernstg added the legacy-area-front-end Legacy: Use area-dart-model instead. label Mar 16, 2020
@johnniwinther johnniwinther added the NNBD Issues related to NNBD Release label Mar 16, 2020
@johnniwinther johnniwinther self-assigned this Mar 16, 2020
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. NNBD Issues related to NNBD Release
Projects
None yet
Development

No branches or pull requests

2 participants