These packages can be specified with the `non-hot-reloadable-package` flag and will be compiled without their members being properly hot reloadable. In exchange, we can perform additional optimizations (such as avoiding levels of indirection/checks in anticipation of their being updated).
This additionally overhauls our memory compiler to use the incremental compiler for proper state-passing hot reload testing + adds tests for this functionality.
Change-Id: Ief65896410f32655f064bd15728703629faa4051
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428343
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
`needsCapture` will ensure that any variables used within an async scope get included in the 'asyncScope' object that's created for that scope.
The variables used to lower Dart late variables in particular get emitted separately. But they can still be used across async scopes so they need the special capture logic as well.
Bug: https://github.com/dart-lang/sdk/issues/60748
Change-Id: I2486fce41f88f186fd799029c2cc59635f7ad8f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429780
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
when they have been rewritten to include hot reload checks. Appears to
fix the issue detected in the dwds test failure.
Issue: https://github.com/dart-lang/webdev/issues/2617.
The new mapping ensures breakpoints applied to the call site in the Dart
source are the call site instead of the the generation check.
Allows the tooling to set a breakpoint on the static method call site,
and then when paused, a step-into will action will advance the program
into the static method.
In the future we will need to reevaluate how we generate code where a
single branch of execution in Dart source produces multiple branches in
JavaScript. I believe right now the tooling will not support creating
breakpoints in both branches even when they both have a mapping to the
same Dart source location.
Change-Id: I1ea0548768d8213dc7eefc332814d3bda7c81535
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427567
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Moves the remaining special case logic out of `_emitMethodCall()`
simplifying the reasoning about where so hot reload soundness checks
can be added.
The original change exposed a latent soundness bug that allowed for
some dynamic getter invocations to be performed without runtime checks.
This reland adds adds a note about the newly enforced runtime checks to
the CHANGELOG.
Change-Id: I387438b8615e60f73dfc5e3b883b8c6d5a9993da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425942
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Previously, code in a closure was compiled with the assumption that
invocations known to be statically sound didn't need any runtime checks
to guaranty soundness.
Now, if the code is retained from an earlier compile and executed after
a hot reload, any assumptions regarding soundness may be invalid.
This change transforms what was an invocation expression into a larger
expression that performs additional checks (similar to a dynamic call).
All checks are skipped if the compile generation matches the current hot
reload generation at runtime.
The soundness checks are handled by a new runtime helper method
`hotReloadCorrectnessChecks`. Any argument expressions are hoisted into
let variables to preserve their evaluation order and to ensure they are
only evaluated once. The helper checks for the existence of the
invocation target, the shape of the method signature, and runtime type
of the passed arguments. Mismatches result in an invocation of
`NoSuchMethod`. Any returned value is cast to the expected static type
of the original invocation.
Change-Id: If925f1a9b475d5ac581b2526403ab7c95753ffef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421640
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
It is possible with the way the library manager is written
today that during a hot restart, a previously defined library
may be initialized. Because the manager uses the same map
to store the initializers for the non-hot restart case and
the hot restart case, we may end up using a newer version of
code in an older generation. Instead, this CL adds a pending
map that stores the hot restart initializers so that we can
later add them to the general map of initializers right before
we call main during a hot restart and during synchronous code.
This should avoid that race condition.
Change-Id: Ib40cbe5bec400035276ebd6fca12efed7077a2b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425190
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
- Sets the name field in the emitted LibraryBundle node to
to the provided module name. We tried to avoid providing a
module name as the notion doesn't map cleanly to library
bundles, but we need to respect the provided module name.
- Emits a call to set the source map for a given library
bundle url. We don't need the full trackLibraries call so we
don't use it.
- Adds a check to avoid redefining libraries outside of a hot
reload or a hot restart. This was handled in the same code
where we emitted trackLibraries calls in the old compiler
for dynamic modules, but we want this for all code, regardless
of whether dynamic modules is enabled. So, add this check in
the library manager instead.
Change-Id: Ic4e86dbfca2b3eefa99cce44fae0973164ab77a5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423620
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Create a new helper `_emitArgumentGroups()` used for the calling
conventions where arguments are not passed in a flattened list.
Towards reducing the variety of code that flows through
`_emitMethodCall()` so hot reload soundness checks can be added when
they are appropriate.
Change-Id: I50ef9c4d54d71408d71b76fc2ca0acb73ef2af08
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422367
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
`visitFunctionInvocation()` and `visitLocalFunctionInvocation()` now
emit code directly.
Towards reducing the variety of code that flows through
`_emitMethodCall()` so hot reload soundness checks can be added when
they are appropriate.
Change-Id: I11e7fef6a454509a18f570bc2a316e81b9e09f2a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422365
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
d8 tests weren't executing code after a hot restart due to several factors:
1) subsequent calls to `main` after a hot restart weren't being added to an event loop.
2) periodic timers, which use `setInterval` weren't updated to check for the hot restart generation.
3) d8's simulated timers run synchronously, which interacts poorly with our async implementation. Periodic timers never cede to the async task that handles changing hot restart generation, so they would run forever whenever an error was thrown.
Changes:
* Added a helper to d8.js that cancels all timers.
* d8 now cancels all timers if an async main registers an error (via a handler on main).
* `setInterval` is now implemented.
* The embedder now accepts a publicly modifiable config object. `capturedMainHandler` and `mainErrorCallback` can be set via this object.
Change-Id: I523752ea69e8fd1f1ec0f6f585a484b670534cfc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421680
Commit-Queue: Mark Zhou <markzipan@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This removes the 'bind' and 'bindCall' methods in our runtime in favor of extending 'tearoff' to support their behavior. Also introduces 'superTearoff' for invoking torn off super members late.
Change-Id: I52ab797558a225c57cc2c6197c7b1bfa7f05a3c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416763
Commit-Queue: Mark Zhou <markzipan@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
When parsing `--define` or `-D` arguments don't split the the value by
commas.
This is consistent with how dart2js handles `-D`, but inconsistent with
how VM handles it.
Example:
void main() {
print(const String.fromEnvironment("FOO"));
}
When compiled with `dart compile js -DFOO="a, b"` and run, dart2js
prints
a, b
VM prints (when compiled to exe)
a
Between these two, I think dart2js' behavior is more common, so we
follow dart2js.
Also update compile_benchmark to avoid splitting a single argument "a b"
into "a" and "b" when parsing the arguments and then splicing them back
before calling `dart2wasm`.
Also update the test runner and ddc batch mode argument parser to handle
splitting quoted arguments in `// dart2jsOption = ...` and the same
options for ddc and dart2wasm, by moving dart2js's `splitLine` to a new
library and reusing it in the test runner and ddc.
Fixes https://github.com/flutter/flutter/issues/164873.
See also https://github.com/dart-lang/sdk/issues/60341 for relevant
future work.
Change-Id: Idbdf69072fa212c8e4a390990577eb5a57b49e8a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415280
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
This new update adds fixes + tests for cross-module super mixins. Super getters can refer to mixins compiled in other modules, so we need to expose all mixin names to make them accessible from their enclosing library.
This is a reland of commit e4c4d0f839
Original change's description:
> [ddc] Overhauling tearoff equality and identity.
>
> Hot reload requires DDC to update how its tearoffs are represented. Tearoffs obey the following conventions:
> * Instance tearoffs are never identical
> * Tearoffs with the same object target and name have the same hash code (even if they resolve to different functions across hot reloads)
> * Two separate tearoffs of the same member are equal
>
> To support this, tearoff equality must not depend on the bound object and method but a composite of the bound object, torn off member name, and the exact class/object from which the member was torn off.
>
> Notable changes:
> * Methods' immediately bound targets are emitted with member signatures. This is required to determine the bound targets for instance and dynamic tearoffs. Bound targets are identified by `libraryUri:class` strings.
> * `applyMixin` passes in a 'true' bound target. This is because mixin applications' members are considered children of their 'on' class (not the mixed in class) wrt equality/hashCode.
> * `bind` is modified to pass in its 'true' bound object to support mixins' super getters.
> * `tearoff` and `staticTearoff` are modified to accept a bound target string (only required for static tearoffs, as they are bound at tearoff-creation-time).
> * Static tearoffs avoid using their bound object for hashcode and equality, as these libraries may be wrapped in proxy objects.
> * Tearoff equality and hashCode are updated to consider bound object, bound name, and its bound method's immediate target.
> * 'noSuchMethod' and 'toString' methods are always accessed through their extension property during signature lookups.
>
>
> Change-Id: Ica5501b6860c605db50aa945bafb6802a7317511
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406723
> Reviewed-by: Nate Biggs <natebiggs@google.com>
> Reviewed-by: Nicholas Shahan <nshahan@google.com>
> Commit-Queue: Mark Zhou <markzipan@google.com>
Change-Id: I645992030f9106c158426412387ddecafc78e3f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415102
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
* Remove accessing nullability through a library node since they no
longer differ depending on the language version.
* Cleanup old comments related to legacy types.
* Cleanup dead code in nullable inference of `.toString()` calls.
Change-Id: I099180e7bab4f0f1ea7cf23a6c0fc015ab0413b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415160
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
This reverts commit e4c4d0f839.
Reason for revert: Breaks are blocking roll into flutter
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8720592021329723649/+/u/run_test.dart_for_web_canvaskit_tests_shard_and_subshard_0/stdout
Original change's description:
> [ddc] Overhauling tearoff equality and identity.
>
> Hot reload requires DDC to update how its tearoffs are represented. Tearoffs obey the following conventions:
> * Instance tearoffs are never identical
> * Tearoffs with the same object target and name have the same hash code (even if they resolve to different functions across hot reloads)
> * Two separate tearoffs of the same member are equal
>
> To support this, tearoff equality must not depend on the bound object and method but a composite of the bound object, torn off member name, and the exact class/object from which the member was torn off.
>
> Notable changes:
> * Methods' immediately bound targets are emitted with member signatures. This is required to determine the bound targets for instance and dynamic tearoffs. Bound targets are identified by `libraryUri:class` strings.
> * `applyMixin` passes in a 'true' bound target. This is because mixin applications' members are considered children of their 'on' class (not the mixed in class) wrt equality/hashCode.
> * `bind` is modified to pass in its 'true' bound object to support mixins' super getters.
> * `tearoff` and `staticTearoff` are modified to accept a bound target string (only required for static tearoffs, as they are bound at tearoff-creation-time).
> * Static tearoffs avoid using their bound object for hashcode and equality, as these libraries may be wrapped in proxy objects.
> * Tearoff equality and hashCode are updated to consider bound object, bound name, and its bound method's immediate target.
> * 'noSuchMethod' and 'toString' methods are always accessed through their extension property during signature lookups.
>
>
> Change-Id: Ica5501b6860c605db50aa945bafb6802a7317511
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406723
> Reviewed-by: Nate Biggs <natebiggs@google.com>
> Reviewed-by: Nicholas Shahan <nshahan@google.com>
> Commit-Queue: Mark Zhou <markzipan@google.com>
Change-Id: Ic5694976260189f7215dfa3c2318e9a0656f0de6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415100
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Hot reload requires DDC to update how its tearoffs are represented. Tearoffs obey the following conventions:
* Instance tearoffs are never identical
* Tearoffs with the same object target and name have the same hash code (even if they resolve to different functions across hot reloads)
* Two separate tearoffs of the same member are equal
To support this, tearoff equality must not depend on the bound object and method but a composite of the bound object, torn off member name, and the exact class/object from which the member was torn off.
Notable changes:
* Methods' immediately bound targets are emitted with member signatures. This is required to determine the bound targets for instance and dynamic tearoffs. Bound targets are identified by `libraryUri:class` strings.
* `applyMixin` passes in a 'true' bound target. This is because mixin applications' members are considered children of their 'on' class (not the mixed in class) wrt equality/hashCode.
* `bind` is modified to pass in its 'true' bound object to support mixins' super getters.
* `tearoff` and `staticTearoff` are modified to accept a bound target string (only required for static tearoffs, as they are bound at tearoff-creation-time).
* Static tearoffs avoid using their bound object for hashcode and equality, as these libraries may be wrapped in proxy objects.
* Tearoff equality and hashCode are updated to consider bound object, bound name, and its bound method's immediate target.
* 'noSuchMethod' and 'toString' methods are always accessed through their extension property during signature lookups.
Change-Id: Ica5501b6860c605db50aa945bafb6802a7317511
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406723
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
Closes https://github.com/dart-lang/sdk/issues/60109
Both getClassMetadata and getClassesInLibrary can be called before
main is called. In order to support this, libraries should be
initialized so that these debugger APIs can inspect them. Similarly,
SDK libraries that are needed before any code can run should be
initialized. In order to support this, they are initialized on the
first initializeAndLinkLibrary call and reinitialized during a
hotRestart (since the libraries are recreated).
Tests are added to evaluate these methods before main is called.
The debugger test helpers are amended to:
- Support breakpoints within the bootstrap script. This is done by
caching the script and querying to see if it has the breakpoint if
the input sources do not.
- Refactor shared test expectation logic.
- Remove an unused method.
Change-Id: I5534d7008436a51243cf51dba01bb8ad06adca69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410602
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
The attached bug shows an issue users have been encountering where a constructor seems to be undefined. It turns out this is because DDC is trying to read the constructor from the wrong library.
This is happening because both 'package:dio' and a sister package 'package:dio_web_adapter' both contain a library with the same path: 'src/adapter.dart'. 'BrowserHttpClientAdapter' the class they are trying to reference is defined in 'package:dio/src/adapter.dart'. However, due to a naming collision, their import is referencing 'package:dio_web_adapter/src/adapter.dart'.
This naming collision happens because of the logic in '_jsLibraryAlias'. By truncating the start of the import URI (i.e. 'dio/' and 'dio_web_adapter/') the two libraries map to the same alias. This alias is then used to as the key in the AMD module export object and since both libraries are in the same module, only the second one gets exported.
This code may have been written with the assumption that libraries from different packages would always be in different modules (in which case the shortened paths shouldn't collide) but this is not the case. The fix is to use the full import URI including the package name.
In writing the attached modular test I discovered another issue that only affects es6 imports. The ScopedId resolver was not considering NameSpecifier as a declaration point for variables. This lead to a similar name collision since the import alias's name was also being derived from a truncated import URI. In the test, both 'f1/foo.dart' and 'f2/foo.dart' were being imported 'as foo'. Now one is 'as foo' and the other is 'as foo$'.
The first issue affects both AMD and es6 while the second issue only affects es6. The modular tests run with es6 so the new test fails if either of these fixes is not in place.
The new DDC module system is not affected by either issue since it doesn't use NameSpecifiers and it uses the full import URI as a string to register libraries rather than a shortened alias.
Tested on TGP and with a local Flutter application.
Bug: https://github.com/dart-lang/sdk/issues/56498
Change-Id: I5bdb945cfbe615874b40e2fc4ebba31b661cf3b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410260
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
* Demonstrates the ability to generate metadata when inspecting the
last accepted and delta components.
* Attach the metadata to the component right before compiling with DDC.
* Use the metadata to avoid deoptimizations in the initial compile
when they are only required to support a hot reload.
* Deletion of all type rules for classes that extend Object in the
initial compile. In the future this should be reduced further to
only the classes that had a hierarchy change in a hot reload.
* Type checks on the return value of getters used to represent
fields.
Change-Id: I2812b564bc3f4d72f005d4bd11fa55ec0eb394ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404940
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Match the behavior of the ddc module system when running the
applications main method when using the new library bundle format.
This passes an empty JavaScript array to the main method which is
good enough to get the benchmarks that use main method args to run.
The API should be updated in a way that allows you to actually pass
a JavaScript array of Strings when starting the program.
Issue: https://github.com/dart-lang/sdk/issues/35113
Change-Id: I1a9aeb1df7afa5948d7f23517b3ac913aaabb62c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/408640
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
MarkZ