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>
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>
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>
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>
Enums are now split between pre and post canonicalization members. A
separate operation during link-time is emitted for every const
enum field. These are required during link time since enhanced enums can have non-trivial type hierarchies.
Example for snippet:
```
enum E {
e1(1), ...
const E(this.i);
final int i;
}
```
Used to emit enum fields as:
```
dart.defineLazy(CT, {
get C1() {
return C[1] = dart.const(Object.setPrototypeOf({
[_Enum__name]: "e1",
[_Enum_index]: 0
i: 1
}, E.prototype));
},
}
```
Now emits them as:
```
// Declaration-time
dart.defineLazy(CT, {
get C1() {
return C[1] = dart.const(Object.setPrototypeOf({
[_Enum__name]: "e1",
}, E.prototype));
},
}
// Link-time
dart.extendEnum(dart.const(Object.setPrototypeOf({
[_Enum__name]: "e1"
}, E.prototype)), {
i: 1,
get index() {
return E.values.indexOf(this);
}
});
```
Change-Id: Id5ce2ec117e59d8daa28df7fe6051e4a7e1a5bc1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404723
Commit-Queue: Mark Zhou <markzipan@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Initially this visitor will reject reloads (by throwing an Exception).
Future changes will add the ability to record data about the delta to
guide the JavaScript compilation decisions.
- Rejects deltas that delete all const constructors from a class
(making it non-const).
- Adds ability to test the visitor directly by compiling components
from source via an in-memory compiler.
- Updates DDC frontend_server compiles to write errors to the output
stream.
Change-Id: Ib318f42d28367416983266a214f97821a38a2913
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401600
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Tearoffs are now represented as a closure that resolves an underlying bound context and property on access. `_boundMethod` and RTI getters must also be evaluated late.
Additionally, we now both canonicalize static methods and tag them with their types at class-declaration time (though lazily) - so that late resolved closures have access to their types.
Some tests have been updated to expect simpler errors. DDC traditionally emits slightly different errors that might aid in debugging.
Change-Id: I1f762b8df45e0766d16dbc8688073768c8bfd233
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401321
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
The async rewriter was ignoring these variables since they were only used for non-user variables that didn't need to be captured. Now that we use these ScopedIds for user variables we need to capture some of them. "userDefined" specifies if the variable should be captured.
Change-Id: I2dbb86e1834982b59883a3d107612ee8f8e2284a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400662
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Some CFE lowerings (e.g. pattern lowerings) result in nested scopes containing VariableDeclarations with the same 'name'. The current DDC transform translates these to the exact same name in JS leading to incorrect semantics.
The `TemporaryId` mechanism automatically renames any variables with the same name that would shadow each other. So we re-use that here to ensure the variables all have a unique name if the CFE hasn't already given them one. If the name is already okay (i.e. not shadowing something else), the name in JS will appear unchanged.
Side note: In a future change perhaps we should rename `TemporaryId`. The general mechanism it implements is more useful than its original intended use.
Fixes: #59613
Change-Id: I708c72528d5df19af48dde01163d375a5588baae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398504
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This is required when a hot reload causes changes to the subtype hierarchy.
This change also adds RTI operations for clearing subtype caches and deleting type rules.
The DDC Embedder also now accesses the RTI library to clear subtype caches on hot reload.
See: #57049
Change-Id: I50a43ce342f23060bc28a3654c2da37c362492b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/394040
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
Tested with test case outlined in bug below. This should be tested via Lasse's tests when he lands them.
For slightly more context we compile code like this:
do {
switch (y) {
case 1:
continue L1;
L1:
case 2:
continue;
}
} while (x);
into something like this:
do {
while (true) {
var labelState = y;
switch (labelState) {
case 1:
labelState = 2;
case 2:
continue; // <-- This now only continues the while (true).
}
}
} while (x);
Bug: https://github.com/dart-lang/sdk/issues/59593
Change-Id: I9b343fd918e48cb44b65e90eab39d70fca4758d4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/397105
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
Type changes across hot reloads need to be performed even if the underlying value has already been initialized.
To avoid extraneous type checks, getters replace themselves with a direct access on their value store on first access.
Also updates our builder to support get/set as property descriptor functions.
Adds some extra conditions that will be cleaned up when pragma support is added (#57049).
Change-Id: I44f4a9e1a6fd8a49aca5e629fb21348170a9d80f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393041
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
- Exposes a minimum set of APIs that are needed by DWDS
a single class Debugger.
- Renames some APIs and modifies the signature to better
align with what's happening and what DWDS actually uses.
Similarly, modifies the internal APIs to handle the new format.
- Modifies expression evaluation test suite to handle the
new module format correctly.
- Modifies LibraryBundleCompiler to emit the right export name.
- Adds/adapts existing tests for the runtime debugger APIs.
With this, all tests within pkg/dev_compiler/test/expression_compiler
pass with the new module format with the exception of two tests within
expression_compiler_e2e_ddc_null_safe_test, which will be fixed
in a future CL.
Change-Id: I296496441ea421ecb57bed3b2e90b92365fef510
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391308
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Removes:
* Unused SDK-specific field emission logic
* after-class-def items, which used to hold lazily emitted classes
* deferred supertypes/classes, which are no longer needed after the new class representation
Change-Id: Ibdaa8d32aecc42c25749cc6a2290ce0bfd4662f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391488
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Mark Zhou <markzipan@google.com>
This restructures all fields as accessor/value-store pairs, providing a level of indirection required for hot reload.
Major updates include:
- All fields (top-level, lazy, virtual, etc.) now have a value store (represented as a private top-level symbol). The value store is initialized on first access for the initializer.
- Lazy value stores are prefixed with '_#v_' to indicate that they are not replaced on a hot reload.
- `declareClass` and `declareTopLevelProperties` are introduced to append classes/members to libraries. These functions extend the 'original' entity/class with fields introduced by a properties object, ignoring the special lazy value stores mentioned above.
- `defineLazy` is replaced with the above operations.
- Virtual/instance fields are still initialized by inside their constructor on first load (so their getter forwards to its value store). On hot reload, however, uninitialized fields are lazily initialized.
- Final fields now use a sentinel value to check for late initialization errors during the initialization loop.
- The JS AST is extended to support class properties.
Change-Id: I5cc3548477d83897273f3b993b304a804754ec0e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386971
Reviewed-by: Nicholas Shahan <nshahan@google.com>
In the expression compiler worker. It appears all uses manually
pass the flag and this is just preparation to remove it in the
future.
Change-Id: Iad0edfbcab28194b4ffcb520db89943c6239753b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388745
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Nicholas Shahan