Skip to content

Kernel mode should retain the existing support we have for Dart API #29989

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
a-siva opened this issue Jun 22, 2017 · 11 comments
Closed

Kernel mode should retain the existing support we have for Dart API #29989

a-siva opened this issue Jun 22, 2017 · 11 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@a-siva
Copy link
Contributor

a-siva commented Jun 22, 2017

Known issues that cause some Dart API functions to not work:

1. imports_field and exports_field in a library object are not initialized correctly

When using the --dfe option I noted that the library objects created in the VM heap do not seem to have the 'imports_' and 'exports_' fields initialized with the various imported/exported library objects. Is this intentional? How does lookup work if these entries are not populated?

For the test listed below, I stop in the debugger after Dart_LoadKernel is called and look at the library object returned and it shows the following:

imports_ field is initialized to an Array object of length 4, with the null object in each element of the array

exports_ field is initialized to the empty_array object.

A lot of functions like Library::LookupObjectAllowPrivate , Library::LookupReExport etc depend on these fields being set.

e.g
file junk.dart contains
import 'junk1.dart';

main() {
imported_main();
exported_main();
}

file junk1.dart contains
library junk1;

export 'junk2.dart';

imported_main() {
print("Import is working");
}

file junk2.dart contains
library junk2;

exported_main() {
print("Export is working");
}

2. Investigate other API calls that do not work

@a-siva a-siva added area-kernel type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jun 22, 2017
@kmillikin
Copy link

kmillikin commented Jun 23, 2017

Yes, that's intentional. At least, it's a consequence of an intentional design.

Kernel is fully resolved, so there is no lookup the way you are thinking of it. That lookup happens when Dart is compiled to Kernel, and Kernel code can directly point to the things it names.

Kernel libraries don't have imports or exports. If we require that Kernel libraries and Dart libraries correspond one to one, then we could represent imports and exports syntactically in Kernel but they would still have no semantic effect. This raises the question of how a transformation that generated libraries would decide on their imports and exports (I suppose if we don't care that they are coherent with the Kernel program, it doesn't matter). And a transformation that combined libraries could no longer represent the original Dart source imports and exports correctly.

At this point, I'd like to figure out what reflective capabilities of Dart source code we should support for the VM's public API. My inclination is that these capabilities should be supported by querying the front end directly (because it knows all about Dart code) and not represented syntactically in Kernel.

Do you think it's possible to come up with a doc that enumerates the reflective capabilities we need?

@a-siva a-siva changed the title kernel reader does not seem to populate the imports_ and exports_ fields in the Library objects created Kernel mode should retain the existing support we have for Dart API Aug 9, 2017
@sivachandra
Copy link
Contributor

For the first round, I will tackle the dartk failures in dart_api_impl_test.cc and debugger_api_impl_test.cc.

@a-siva a-siva self-assigned this Nov 13, 2017
@a-siva a-siva added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. and removed area-kernel labels Nov 13, 2017
@a-siva a-siva added this to the 2.0-alpha milestone Nov 13, 2017
@a-siva a-siva assigned bkonyi and unassigned sivachandra Feb 7, 2018
@a-siva
Copy link
Contributor Author

a-siva commented Feb 16, 2018

Most of the API works, will not close this issue until we make sure all the status file entries in tests related to this are verified.

@a-siva a-siva modified the milestones: 2.0-alpha1, 2.0-alpha2 Feb 16, 2018
@JekCharlsonYu JekCharlsonYu modified the milestones: 2.0-alpha2, I/O Beta 3 Feb 23, 2018
@dgrove
Copy link
Contributor

dgrove commented Mar 30, 2018

any updates on this, @a-siva ?

@a-siva
Copy link
Contributor Author

a-siva commented Mar 30, 2018

We still have some status file entries that turn off API tests in Dart 2 mode, bkonyi is working through the list.

@dgrove dgrove modified the milestones: Dart2 Beta 3, Dart2 Beta 4 Apr 12, 2018
@a-siva
Copy link
Contributor Author

a-siva commented May 3, 2018

@bkonyi is it safe to say that we now support all of the Dart API surface in Dart 2?

@bkonyi
Copy link
Contributor

bkonyi commented May 3, 2018

For the most part all of the tests for the Dart API are passing or marked as SkipByDesign (for features not supported in Dart 2). However, there's still a few tests which need updating but aren't high priority:

@bkonyi bkonyi modified the milestones: Dart2Beta4, Dart2Stable May 3, 2018
@dgrove
Copy link
Contributor

dgrove commented Jun 22, 2018

@bkonyi - I see that #33043 is now targeted for Dart2.1 . Is the rest of this issue required for Dart2Stable?

@bkonyi bkonyi modified the milestones: Dart2Stable, Dart2.1 Jun 25, 2018
@bkonyi
Copy link
Contributor

bkonyi commented Jun 25, 2018

Sorry for the delay. I had discussed with @a-siva about #33041 and I think the conclusion we came to was that while we would prefer this made it into Dart2Stable, it's not essential. That issue is blocked by #33495, so I think we should be good to go (unless @a-siva has a different opinion). I'll move this issue to Dart2.1 to keep tracking the unresolved issues.

@dgrove
Copy link
Contributor

dgrove commented Oct 9, 2018

Still targeted for 2.1?

@a-siva
Copy link
Contributor Author

a-siva commented Oct 9, 2018

We have covered the Dart API surface area in kernel mode and so this generic bug can be closed. We have individual bugs for some cases where there are bugs.

@a-siva a-siva closed this as completed Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants