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.

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

[Breaking Change Request] [vm] Deprecate and remove dart native extensions #45451

Closed
1 task done
mraleph opened this issue Mar 24, 2021 · 45 comments
Closed
1 task done
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. breaking-change-request This tracks requests for feedback on breaking changes P2 A bug or feature request we're likely to work on

Comments

@mraleph
Copy link
Member

mraleph commented Mar 24, 2021

Summary

Dart Native Extensions is a mechanism for loading native code into a running Dart program and binding Dart methods to native implementatations.

It consists of the following parts:

  • dart-ext:... import scheme which instructs VM to load a native library;
  • native keyword which specifies that a function has a native implementation;
  • Dart VM C API (as defined in runtime/include/dart_api.h) which can be used to reflectively inspect and mutate the state of the Dart program;

Currently native extensions only really work in a standalone Dart VM - Flutter embedder hides VM C API symbols and does not allow developers to interact with Dart through Dart VM APIs.

Relience on C API means that:

  • Native extensions are not very efficient for fine-grained binding.
  • Reflective access through Dart VM C API is also incompatible with AOT (requires careful usage of @pragma('vm:entry-point') which in turn disables treeshaking).

dart:ffi was designed to not have these problems - be both more effecient and more AOT friendly. dart:ffi works in standalone VM as well as Flutter.

From our perspective it does not make sense to maintain two completely different approaches to binding Dart code to native especially given that one of them is obviously superior. We don't want more packages using native extensions to appear in the ecosystem and we want to push all new Dart users to use dart:ffi.

We propose the following:

  • In Dart 2.14 issue a deprecatin warning whenever analyzer or CFE see dart-ext: import.
  • In Dart 2.15 remove support for dart-ext: imports and hide Dart VM C API symbols from standalone dart binary shipped with SDK

Impact

All packages that use dart-ext: will be broken by the release. We will work to estimate the impact, but we consider it to be very small because none of the Flutter compatible packages will be affected, as Flutter does not support this mechanism in the first place.

TODO

@mraleph mraleph added the breaking-change-request This tracks requests for feedback on breaking changes label Mar 24, 2021
@mraleph
Copy link
Member Author

mraleph commented Mar 24, 2021

/cc @a-siva @mkustermann

@devoncarew devoncarew added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Mar 24, 2021
@a-siva
Copy link
Contributor

a-siva commented Mar 24, 2021

Adding the March milestone for the deprecation warning in 2.14 from analyzer and CFE when dart-ext import is seen.

In 2.15 we should also remove samples/sample_extension

@a-siva a-siva added this to the March Beta Release milestone Mar 24, 2021
@a-siva a-siva added area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Mar 24, 2021
@a-siva
Copy link
Contributor

a-siva commented Mar 24, 2021

Also added area-analyzer and area-front-end labels for the warning work in 2.13

@devoncarew
Copy link
Member

A deprecation warning wouldn't be much work, but we probably want to discuss adding new work to a release a few working days before a branch cut.

@mraleph
Copy link
Member Author

mraleph commented Mar 24, 2021

@devoncarew I have updated versions to 2.14 and 2.15 and moved the milestone. No intention to fall over ourselves with this.

@devoncarew devoncarew added the P2 A bug or feature request we're likely to work on label Mar 29, 2021
@blagoev
Copy link
Contributor

blagoev commented Mar 31, 2021

Does Dart SDK follow semver versioning?

@mraleph
Copy link
Member Author

mraleph commented Mar 31, 2021

@blagoev Not really.

@franklinyow
Copy link
Contributor

Ping @vsmenon @matanlurey @Hixie for approval

@Hixie
Copy link
Contributor

Hixie commented Apr 9, 2021

Fine by me since this doesn't affect Flutter anyway. :-)

@vsmenon
Copy link
Member

vsmenon commented Apr 9, 2021

lgtm

@franklinyow
Copy link
Contributor

Approved

@franklinyow
Copy link
Contributor

@mraleph
Copy link
Member Author

mraleph commented Apr 20, 2021

@devoncarew @johnniwinther does it sound reasonable to have analyzer produce deprecation warnings in 2.14? I want to get versions right in the announcement.

@johnniwinther
Copy link
Member

SGTM

Do you have wording to use in the deprecation warning?

@mraleph
Copy link
Member Author

mraleph commented Apr 20, 2021

@johnniwinther I suggest "Dart native extensions are deprecated and will be removed in 2.15 release of Dart SDK. Migrate to using FFI instead (https://dart.dev/guides/libraries/c-interop)". @mit-mit any thoughts?

@srawlins
Copy link
Member

What is the plan for moving internal code off of dart-ext? I see a few cases when I search lang:dart import\ ['"]dart-ext in code search.

@mraleph
Copy link
Member Author

mraleph commented Apr 20, 2021

I have reached to some of the internal owners and working on planning migration

@vsmenon
Copy link
Member

vsmenon commented Aug 4, 2021

@mraleph - does this still need to be open?

@a-siva
Copy link
Contributor

a-siva commented Aug 4, 2021

We still have samples/sample_extension in the repo, are we good to go on deleting it. I can submit a PR deleting the directory

@devoncarew
Copy link
Member

I believe we were going to deprecate in 2.14 (shipping soon) and remove in 2.15.

@a-siva
Copy link
Contributor

a-siva commented Aug 4, 2021

Yes the removal will be in the first beta of 2.15

@GavinRay97

This comment has been minimized.

@mraleph

This comment has been minimized.

@mraleph
Copy link
Member Author

mraleph commented Aug 31, 2021

@johnniwinther @srawlins could you make changes to CFE and analyzer to start rejecting dart-ext: imports? (promote the current deprecation warning to a compilation error).

@lrhn
Copy link
Member

lrhn commented Sep 1, 2021

At the time we remove this feature, we should also disallow using native as a function body.
Code like

int systemRand() native "SystemRand";

should become a parsing error, not a run-time "native function 'SystemRand' (0 arguments) cannot be found" error.
(We can be gentle with the compile-time error if we recognize the intent, but it should be a compile-time error).

@mraleph
Copy link
Member Author

mraleph commented Sep 1, 2021

@lrhn yeah, I will do few separate changes to replace native with an annotation.

dart-bot pushed a commit that referenced this issue Sep 2, 2021
Per breaking change #45451 we are removing support for dart-ext:
style native extensions from the Dart VM.

This CL removes the associated VM code, tests and samples. It also ports
a single test which used dart-ext: import to use FFI instead.

TEST=ci

Bug: #45451
Change-Id: Iae984bce32baf29a950b5de1323939006a217b94
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212050
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
dart-bot pushed a commit that referenced this issue Sep 3, 2021
#45451

Change-Id: Ieed7c4a200b0480975715a149596b1f5ea733814
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212002
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>
@mraleph
Copy link
Member Author

mraleph commented Sep 7, 2021

VM and CFE has removed support for dart-ext. Analyzer work pending.

Removal of native syntax is tracked in #28791

@devoncarew
Copy link
Member

@mraleph - do you know what the remaining work here is for the stable release? If there is work remaining, perhaps track using sub-issues and update the first comment in this one to be a checklist / meta issue.

@mraleph
Copy link
Member Author

mraleph commented Oct 8, 2021

The only missing piece is promotion of warning / info to error in the analyzer for dart-ext: imports. Will file an issue.

@mraleph
Copy link
Member Author

mraleph commented Oct 13, 2021

This is done.

@mraleph mraleph closed this as completed Oct 13, 2021
copybara-service bot pushed a commit that referenced this issue Oct 19, 2022
The sample_extension has been deleted a while ago.

Bug: #45451
Change-Id: I69400018345d47f3b078d563246f36412eb3472b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264840
Commit-Queue: Slava Egorov <[email protected]>
Auto-Submit: Alexander Thomas <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. breaking-change-request This tracks requests for feedback on breaking changes P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests