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.

Sign up for GitHub

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

Jump to bottom

[CFE] Implement changes to null safety checking for local variables #43099

Closed
leafpetersen opened this issue Aug 19, 2020 · 6 comments
Closed

[CFE] Implement changes to null safety checking for local variables #43099

leafpetersen opened this issue Aug 19, 2020 · 6 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. NNBD Issues related to NNBD Release type-enhancement A request for a change that isn't a bug
Projects
Dart web null safety

Comments

@leafpetersen
Copy link
Member

Small set of changes to the errors and warnings around local variables landed here.

Tests for the the definite assignment based errors landed here.

Tests for the inference and promotion section to follow.
@leafpetersen leafpetersen added area-front-end Use area-front-end for front end / CFE / kernel format related issues. NNBD Issues related to NNBD Release labels Aug 19, 2020
@leafpetersen leafpetersen mentioned this issue Aug 19, 2020
Closed
@sgrekhov sgrekhov mentioned this issue Aug 26, 2020
Closed
@franklinyow franklinyow added the type-enhancement A request for a change that isn't a bug label Sep 3, 2020
@leafpetersen leafpetersen mentioned this issue Sep 3, 2020
Closed
@leafpetersen
Copy link
Member Author

Note that I believe these tests should be fixed by this work as well:

co19/Language/Variables/final_t04
co19/Language/Variables/final_t05
co19/Language/Variables/final_t06
co19/Language/Variables/final_t07

@leafpetersen
Copy link
Member Author

These tests should also be fixed with this work:

language/final/syntax_test/01
language/final/syntax_test/02

@leafpetersen leafpetersen mentioned this issue Sep 3, 2020
Closed
@nshahan nshahan added this to Related issues to track in Dart web null safety Sep 3, 2020
@leafpetersen
Copy link
Member Author

@johnniwinther I believe this issue covers the tests you have marked with "Late assignment of final variable: Not sure if spec is ready yet." in our tracking spreadsheet. As far as I know, the spec for this is complete and landed, are there open issues I'm forgetting?

@johnniwinther
Copy link
Member

Thanks for the clarification. At this point we're then waiting for the flow analysis to be updated according to the spec. After that we might have some additional integration work on the CFE side.

@leafpetersen
Copy link
Member Author

New tests for the inference and initialization portion landed here

Failing: language/nnbd/inference/variable_initialization_promotion_test
Passing: language/nnbd/inference/variables_initialized_to_null_test

@stereotype441 stereotype441 mentioned this issue Sep 27, 2020
Merged
12 tasks
Dart web null safety automation moved this from Related issues to track to done Oct 1, 2020
dart-bot pushed a commit that referenced this issue Oct 1, 2020
Revert "Flow analysis: promote to non-nullable on initialization"
b4f49dc 
This reverts commit 6a1c54e.

Reason for revert: Triggers test failure:
https://ci.chromium.org/p/dart/builders/ci.sandbox/vm-kernel-precomp-linux-debug-x64/8247

Original change's description:
> Flow analysis: promote to non-nullable on initialization
>
> When flow analysis encounters a variable declaration of the form `T? x
> = expr;`, if the type of `expr` is `T`, then the variable is
> immediately promoted to type `T`.
>
> Fixes #43099.
>
> Change-Id: Ia206fe0d50e2fdd9bdf637e13c85633d8490dbcc
> Bug: #43099
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/163841
> Commit-Queue: Paul Berry <paulberry@google.com>
> Reviewed-by: Bob Nystrom <rnystrom@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

TBR=paulberry@google.com,scheglov@google.com,rnystrom@google.com,johnniwinther@google.com

Change-Id: I8549b48a734f527194ce11d82235b9d5c6e58185
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #43099
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165564
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
@stereotype441
Copy link
Member

Reopening since https://dart-review.googlesource.com/c/sdk/+/165564 reverted the fix

@stereotype441 stereotype441 reopened this Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. NNBD Issues related to NNBD Release type-enhancement A request for a change that isn't a bug
Projects
Dart web null safety
  
done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stereotype441 @leafpetersen @johnniwinther @franklinyow