ddc adds null checks on return values of non-nullable native APIs #42535
Comments
sigmundch
added
area-web
|
@sigmundch also here, blocker for beta? |
|
Similar to #42536 - we'd like it as part of beta if at all possible. This is less critical than the dart2js one because DDC is mostly used together with chrome, which is where most of the non-nullable bits are derived from, but I'd prefer to keep it. |
Bug: #42535 Adds a runtime flag to ddc to enable runtime checks on native APIs. In the case where an API is typed non-nullable but returns a null value, throws a null assertion error. Change-Id: I4d5d7529ba28d9308687dad5d51f1b9c71274455 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162461 Reviewed-by: Nicholas Shahan <nshahan@google.com> Commit-Queue: Srujan Gaddam <srujzs@google.com>
|
Is this done? |
|
We discovered through testing some changes that need to be made for this bug, so it's still in progress. It is OK if this slips and |
|
Moved it to the next milestone |
|
Any update? Thx |
Bug: #42535 Native null-checks are moved to member definitions, and native member accesses are changed to use this indirection in the case where the member type can be checked. Change-Id: I499bb3a4f6a66021dd0ab7930a55e7233c1ce020 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166785 Reviewed-by: Nicholas Shahan <nshahan@google.com> Reviewed-by: Sigmund Cherem <sigmund@google.com> Commit-Queue: Srujan Gaddam <srujzs@google.com>
This reverts commit e02085b. Reason for revert: This seems to break an internal test (http://b/171029249). Original change's description: > [ddc] Move native null-checks to definitions > > Bug: #42535 > > Native null-checks are moved to member definitions, and native > member accesses are changed to use this indirection in the case > where the member type can be checked. > > Change-Id: I499bb3a4f6a66021dd0ab7930a55e7233c1ce020 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166785 > Reviewed-by: Nicholas Shahan <nshahan@google.com> > Reviewed-by: Sigmund Cherem <sigmund@google.com> > Commit-Queue: Srujan Gaddam <srujzs@google.com> TBR=sigmund@google.com,nshahan@google.com,srujzs@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: #42535 Change-Id: I145ff11a18374d0d63e2042956ad53ea0939baf7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/168345 Commit-Queue: Michal Terepeta <michalt@google.com> Reviewed-by: David Morgan <davidmorgan@google.com> Reviewed-by: Michal Terepeta <michalt@google.com>
|
@sigmundch It's been reverted. Who's working on this one? |
|
@srujzs is pushing forward with the original change, but will make the behavior only to be enabled on libraries that are opted in. This strategy doesn't change the essence of the change, but will make it less disruptive and will allow the roll to happen without issues. There are a couple changes involved here (relanding the change, adding a flag), but our expectation is that we'll manage to land it this week. |
|
@srujzs any update? |
|
More info in #42536 that's directly related. Adding on to that, we're planning on only including this in sound null-safety for now with maybe the potential for opt-in unsound null-safety for DDC only. My goal is to land CLs related to this ASAP. |
Bug: #42536 Bug: #42535 Since ddc would require a potentially breaking change to emit these checks in unsound mode without opt-in and it's currently not possible to emit these checks only in opt-in with dart2js, both are changed to only emit checks in sound mode. In ddc, calling convention is changed conditionally on sound mode as well to avoid emitting unnecessary code in unsound mode. Change-Id: I42f7bb5a53550f4ee5412fbbbfb6ca533c393e96 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169247 Reviewed-by: Nicholas Shahan <nshahan@google.com> Commit-Queue: Srujan Gaddam <srujzs@google.com>
No description provided.
The text was updated successfully, but these errors were encountered: