Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@goderbauer
Copy link
Member

Following the lead of dart-lang/sdk@1755f89, this adds the final class modifier to all subclasses of Struct, Union, Opaque, and AbiSpecificInteger to unblock the dart roll (#40426).

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Mar 19, 2023
Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

None of the subtypes of Opaque or Struct may be sub typed again, so final is indeed the right keyword here.

@goderbauer goderbauer requested a review from zanderso March 20, 2023 16:09
@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 20, 2023
@goderbauer goderbauer merged commit 41225fe into flutter:main Mar 20, 2023
@goderbauer goderbauer deleted the tryDartFix2 branch March 20, 2023 17:11
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 20, 2023
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Mar 21, 2023
This is a reland of commit 1755f89

Can land after (or with) the Flutter PR:
flutter/engine#40434

Original change's description:
> [vm/ffi] Add class modifiers
>
> Adds class modifiers to `dart:ffi`.
>
> Migrates all user-defined subclasses of `Struct`, `Union`, `Opaque`,
> and `AbiSpecificInteger` to be `final class`es.
>
> Does not remove the manual error checking, so some errors will show up
> twice now in language version 3.0. In language version <3.0, only the
> FFI-specific error will show up.
>
> In a follow-up CL, we will try to make the language-errors to show up
> also <3.0 so that we can remove the FFI-specific errors.
>
> Examples of duplicated errors:
> pkg/analyzer/test/src/diagnostics/subtype_of_ffi_class_test.dart
>
> TEST=pkg/analyzer/test/ (for the analyzer)
> TEST=pkg/front_end/testcases/ (for the CFE)
> TEST=test/ffi/ (for the VM)
>
> CoreLibraryReviewExempt: No need for dart2js to review.
> Bug: #51683
> Change-Id: I2964ceccb7db59fbdaf6be5319f5e4ec2dabe0f3
> Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-win-release-try,pkg-mac-release-try,vm-precomp-ffi-qemu-linux-release-riscv64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-android-debug-arm-try,vm-reload-rollback-linux-debug-x64-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289223
> Reviewed-by: Johnni Winther <[email protected]>
> Reviewed-by: Devon Carew <[email protected]>
> Reviewed-by: Brian Wilkerson <[email protected]>
> Reviewed-by: Jackson Gardner <[email protected]>
> Reviewed-by: Lasse Nielsen <[email protected]>
> Commit-Queue: Daco Harkes <[email protected]>

TEST=pkg/analyzer/test/ (for the analyzer)
TEST=pkg/front_end/testcases/ (for the CFE)
TEST=test/ffi/ (for the VM)
CoreLibraryReviewExempt: No need for dart2js to review.
Bug: #51683
Change-Id: I2ee3f0ac31d4162068a2346a06320029b2263ee2
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-win-release-try,pkg-mac-release-try,vm-precomp-ffi-qemu-linux-release-riscv64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-android-debug-arm-try,vm-reload-rollback-linux-debug-x64-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289781
Reviewed-by: Devon Carew <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants