VM does not support deferred libraries in Dart2
matanlurey opened this issue · 30 comments
I ran into this while debugging test failures internally that expect this functionality.
Here is a simple reproduction case:
// deferred_lib.dart
final aNumber = 1234;// deferred_lib_test.dart
import 'package:test/test.dart';
import 'deferred_lib.dart' deferred as deferred_lib;
void main() {
test('should throw on access to defererd library', () async {
// In Dart1 this test passes as-is.
// With --preview-dart-2 + Dart VM version: 2.0.0-dev.55.0 this fails:
//
// Expected: throws anything
// Actual: <Closure: () => int>
// Which: returned <1234>
expect(() => deferred_lib.aNumber, throwsA(anything));
await deferred_lib.loadLibrary();
expect(deferred_lib.aNumber, 1234);
});
}I realize there might not be a convincing reason to "really" support deferred loading in the command-line VM in Dart 2 (since it isn't really supportable in Flutter, and I don't think anyone else downloads Dart over the network i.e. Dartium-style).
@srawlins pointed me to:
... which makes me think that this was (intentionally?) never implemented.
/cc @keertip as this is blocking --preview-dart-2 internally, unless I disable the test.
... it turns out this code (and test) is unused, so I can safely delete it. This is NOT a P1.
We should clarify whether or not deferred loading is supported though, and hopefully fail better.
Currently the front end loads all libraries eagerly and hence the first check does not throw. It is not clear if there are plans for the front end to support deferred libraries, maybe @kmillikin could comment on that.
I agree that the front end should produce a better error message and fail without executing any of the code if we choose to not support deferred libraries.
Another option is what DDC does, which is eagerly load everything, but prevent access to the deferred library until .loadLibrary() is called. This gives you the option of implementing it in the future without changing the semantics.
/cc @vsmenon
This is how the Dart1 VM worked, does DDK support this behavior because it would require support from the FE for that too.
@kmillikin will update the issue with details on the kind of support provided in kernel for recognizing libraries that are deferred loaded, the VM can then flag these libraries appropriately and mark the libraries as loaded only after LoadLibrary succeeds.
I don't believe this needs to be fixed for Dart2Stable. @a-siva @matanlurey what are your thoughts?
If deferred loading no longer works in the Dart VM, I think just documentation as such is probably fine:
Deferred loading is currently only enabled for Dart4Web products (DDC, Dart2JS).
When used in the standalone VM, calling
loadLibrary()is optional and has no effect on the runtime (the code is not actually deferred loading). We may explore re-adding this functionality in the future.
... and enough to close this issue. Thoughts?
The other option is the VM failing to compile or issuing a warning is deferred loading is used. I'd prefer that, but I'd understand if its not feasible.
Let's go with documentation for Dart2Stable. /cc @kwalrath Can you add this documentation? Once that's done, we'll move the actual changes to the VM out of Dart2Stable.
To be honest, I'd prefer that we don't go the documentation route: we want consistent semantics, and doing what the Dart1VM seems totally reasonable to me.
I thought that the deferred loading support we added to the FE for dart2js would be practically the same than what the VM needs to implement its checks. In particular, the FE generates expressions for loadLibrary and checkLibraryIsLoaded whenever the code accesses a deferred element. It also handles tearoffs of the loadLibrary method automatically.
@a-siva @kmillikin - is there something else missing that you may need for the VM?
@dgrove & @sigmundch: Should I document this or not?
My preference would be not to document. That being said, this really depends on the remaining work needed to fill the gap in the VM, so I'd wait to hear from @a-siva @kmillikin first before we decide.
@dgrove - do you agree?
is this feature required for Dart2 stable?
We discussed this at todays's scrum meeting and decided to do do the following:
if so fix when it is convenient, but this is no longer blocking Dart2Stable
I'm guessing that the fix will mean code that runs in the VM today will stop working after the fix? Will we consider that non-breaking even though it breaks people?
I'm guessing that the fix will mean code that runs in the VM today will stop working after the fix?
Yes, you can "silently" start using deferred libraries without using .loadLibrary().
correct - that's the main reason I hope the fix is easy and we can avoid the breaking-change. There is a general belief that it is unlikely because almost all users of deferred loading are on the web only.
It sounds like I should say that there's currently a difference but say "don't depend on it" because it's likely to change. And I can point to this bug for details.
I'm wondering if this is worth spending cycles on considering Flutter doesn't support deferred libraries and the Dart 1 VM and DDC only "fake" support deferred loading. It makes sense for DDC to do this since dart2js does actually support deferred loading, but I'd argue that treating the deferred keyword and loadLibrary() calls as noops in the VM is sufficient. Thoughts?
I happen to agree with @bkonyi. The only potential issue is folks writing cross-platform libraries where the implementation of .loadLibrary() varies depending on whether it is running in the web or the VM. Perhaps documentation is enough here, though.
Anyone else have an opinion on this? I'll mention @sigmundch explicitly since he had an opinion on this before.
Maybe reach out to Fuischa potentially or other internal VM partners @bkonyi?
(I don't know if they have any plans to do ephemeral loading in this fashion or not)
@sigmundch is out of the office for quite awhile - we'll need to make the decision without him.
@zanderso @rmacnak-google do either of you happen to know if Fuchsia is planning on using deferred loading?
Discussed with @zanderso + @rmacnak-google offline and it doesn't sound like Fuchsia currently has any plans to use deferred loading and they seem to be on board with treating deferred loading operations as noops in the VM (feel free to correct me if I'm wrong here). I'm going to remove the Dart 2.1 milestone for this issue as this is low priority without any customers currently needing this in the VM.
I'll update the note at https://www.dartlang.org/guides/language/language-tour#deferred-loading to not say that we expect this bug to be fixed soon.