Change-Id: I8e7dc1d6241783d47c1e0b62fe039a7fef16ca44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256000
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
In AOT we can have unboxed fields that will not be visited by GC
visitors and as such are also not reported as <obj>.references when
producing heap snapshots.
Though the heapsnapshot's <class>.fields will contain entries
for unboxed fields, which causes the fields index calculation to be
incorrect.
This can cause various confusing things, e.g. incorrectly caluclated
retaining paths, since the <class>.fields.index doesn't match the
index in <obj>.references.
To fix this this CL will change the visiting code to emit dummy
references for unboxed fields.
The written test uncovered a few other issues:
- report type_arguments and native_fields in <class>.fields
=> the visitors already visit those fields
- fix `Class::next_{host,target}_field_offset` for classes
with native fields
=> this will also shrink size of `NativeWrapperClass` subclasses
- ensure classes with vm-defined layout are finalized
=> we assume objects should only exist in heap if their class
is finalized
- fix `FfiNativeFunction` type_argument offset
- fixes for object fields list
=> add missing entries, fix existing entries
Fixes https://github.com/dart-lang/sdk/issues/49711
TEST=vm/dart{,_2}/heap_snapshot_regress_49711_test
Change-Id: I8e25680b2c0c8c49caafbb8da57b0b6419e89c4f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255814
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The migration of the migration tool was done hastily (because it was
not expected to need much more maintenance) so it contains a lot of
nullable fields that don't need to be nullable. I've been cleaning
them up as I run across them in order to unblock other work. This is
one such cleanup.
Bug: https://github.com/dart-lang/sdk/issues/49601
Change-Id: I68413e23f565bc9d38af3345d28059d734a3c689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256021
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
- Variable types are no longer specified in the call to the `Var`
constructor; they are now specified in the call to `declare`. This
paves the way for supporting variable pattern syntax, in which a
single variable might appear in multiple variable patterns, and have
its type specified in each pattern. The properties `isFinal` and
`isLate` are also moved to `declare` for consistency.
- Variables with inferred types are now specified by simply not
including a type in `declare`; it's no longer necessary to specify
`isImplicitlyTyped: true`.
- `declare` now supports an `expectInferredType` argument to allow the
inferred type of an implicitly typed variable to be tested.
- The tests now check that variables are assigned a type before flow
analysis requests it; previously this was not tested, and the flow
analysis tests sometimes did things in the wrong order. (The
analyzer and CFE have always done this in the proper order though).
- The tests now support some of the crazy types that arise during type
parameter promotion, e.g. they can now distinguish `(T&int)?` from
`T&(int?)`.
- Flow analysis tests now properly replicate the analyzer and CFE
behaviors for converting the static type of an initializer
expression to the corresponding inferred variable type: (a) `Null`
is converted to `dynamic`, and (b) type parameter promotions are
dropped.
Note that this last behavior (dropping type parameter promotions) has
a lot of subtleties, and I'm not convinced the CFE and analyzer do it
soundly in all cases (I've already found one such soundness bug:
https://github.com/dart-lang/sdk/issues/49691). In a later CL, I plan
to add a more thorough set of language tests to verify that we don't
have other lurking soundness issues.
Change-Id: I6f2cd20db1f07b34e0ad4e7002351c8de846b125
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255600
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Additionally, fixed the template for html_dart2js so the generator would generate the correct code.
Change-Id: I2172bb10db413169b86690faa027365ff72e3d68
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255763
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Several unusual constructs that lead to unreachable code are now
recognized by flow analysis:
- Control flow after an expression of the form `e ?? other` or `e ??=
other`, where `e` has static type `Null` and `other` has static type
`Never`, is considered unreachable.
- Control flow predicated on an expression of the form `e is Never`
evaluating to `true` is considered unreachable.
- Control flow predicated on an expression of the form `e is! Never`
evaluating to `false` is considered unreachable.
- Control flow on the RHS of a null-aware access such as
`e?.property...`, `e?.property = ...` or `e?.method(...)`, where `e`
has static type `Null`, is considered unreachable (Note: this can
arise in the presence of extension methods).
Previously, these behaviors only took effect if `e` was a reference to
a local variable.
Note: the change to `regress/issue_31180` is because I’ve corrected
the behavior of implicit temporary variables to not undergo a type
change from `Null` to `dynamic`, so the dead code part of `null?[1]`
is now erroneous. (I had to make this change in order for the last
bullet above to work properly; without it, the type change to
`dynamic` prevents flow analysis from recognizing that the code to the
right of `?.` is unreachable.) There's no behavioral change to
correct code, but I've captured the behavioral change to incorrect
code in
`tests/language_2/null_aware/null_aware_index_on_null_error_test.dart`.
Bug: https://github.com/dart-lang/sdk/issues/49635
Change-Id: I8b24b3b040a34f897c0b61dcb9bd105be6d0af6d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251280
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Previously, tree shaker conservatively treated such fields as used if
constructor of the class was used.
Tree shaker would still keep late final instance fields without
initializer as it doesn't track how many times the field was
assigned/initialized and repeatitive assignment to a late final field
without initializer throws an exception.
TEST=pkg/vm/testcases/transformations/type_flow/transformer/write_only_field3_nnbd.dart
Change-Id: I859a4e864e2186e754ba4387a50d1ad9a901e6ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255542
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Enum index and name instance fields were moved to a base class (_Enum)
and tree shaker no longer needs to keep instance fields of enum classes.
TEST=ci
Change-Id: I2426a0bc37f11e51ca00aa24f981e4663d5fb358
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255543
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This is a reland of commit f5228127f8
Original change's description:
> [vm/compiler] Optimize switch statements
>
> Switch statements that either contain only integers or only enum values of the same type can be optimized.
>
> Depending on the number of switch expressions and the number of holes that the range of switch expressions contains, either a binary search or a jump table is used.
>
> TEST=runtime/test/vm/dart{,_2}/optimized_switch
> TEST=tests/language{,_2}/switch
>
> Fixes: https://github.com/dart-lang/sdk/issues/49585
>
> Co-authored-by: Gabriel Terwesten gabriel@terwesten.net
>
> Change-Id: I62dcdb7843107f03de7e468c60b4db52ec78f676
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253787
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
TEST=runtime/test/vm/dart{,_2}/optimized_switch
TEST=tests/language{,_2}/switch
Change-Id: Ie96e5ff41dce33e2425ba9ad15ecd931d09bec6e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255700
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This is a reland of commit 9700458975
Original change's description:
> [vm/compiler] Initial implementation of IL binary serialization
>
> This change adds binary serialization/deserialization of flow graphs.
> It supports all IL instructions and certain objects which can be
> referenced from IL instructions. IL binary serialization is a useful
> machanism which would allow us to split compilation into multiple parts
> in order to parallelize AOT compilation.
>
> The program structure (libraries/classes/functions/fields) is not
> serialized. It is assumed that reader and writer use the same
> program structure.
>
> Caveats:
> * FFI callbacks are not supported yet.
> * Closure functions are not re-created when reading flow graph.
> * Flow graph should be in SSA form (unoptimized flow graphs are not
> supported).
> * JIT mode is not supported (serializer currently assumes lazy
> linking of native methods and empty ICData).
>
> In order to test IL serialization, --test_il_serialization VM option is
> added to serialize and deserialize flow graph before generating code.
TEST=vm/dart/splay_test now runs with --test_il_serialization.
TEST=Manual run of vm-kernel-precomp-linux-debug-x64-try with
--test_il_serialization enabled (only ffi tests failed).
TEST=gcc build on dart-sdk-linux-try bot.
Issue: https://github.com/dart-lang/sdk/issues/43299
> Change-Id: I7bbfd9e3a301e00c9cfbffa06b8f1f6c78a78470
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254941
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
Change-Id: I64ff9747f761496a096371e490ef070a14023256
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255840
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Procedure names were used unconditionally to generate 'unique' fields.
But the names were equal for getters and setters. This CL adds a kind
postfix.
TEST=pkg/vm/testcases/transformations/ffi/ffinative.dart
TEST=pkg/vm/test/transformations/ffi_test.dart
Closes: 49770
Change-Id: I5c99da4ee50852bdfd67fc999a2b659d050a48f8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255995
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
This adds the flag `isEnumElement` to the `Field` node and uses it
to recognize enum elements for the exhaustiveness computation on
switch statements.
Closes#49697
TEST=pkg/front_end/testcases/general/issue49697/main.dart
Change-Id: I21852f3b063329e043639c825054f538c8ed6536
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255808
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
If a parameter of a public method is accessed using null-aware access
(`?.`), the migration tool now considers that to be an indication that
the parameter should be nullable.
Rationale: when migration doesn't have access to the entire code base,
the migration tool's approach of propagating nullability forward
through the program doesn't always work, because it's possible that
the sources of nulls are not visible to the migration tool. This has
often resulted in the migration tool marking a function parameter as
non-nullable, in spite of the fact that the use of `?.` in the method
body makes it clear that it's intended to be nullable.
This new heuristic is only applied to public methods; for private
methods there's no chance of there being callers outside of the code
that's immediately visible to the migration tool, so the problem
doesn't arise. For local functions and closures, the situation is
less clear-cut, but of the examples I've looked through so far in
Google's internal code base, it seems like more often than not, the
better behavior is to continue erring on the side of non-nullability.
Bug: https://github.com/dart-lang/sdk/issues/49601
Change-Id: I76531591ce0b3eb9fe62273130aa45eb4ff6d456
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253864
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Instead of using Expando, we store this data in an object that
lives as long, as we need it to compute diagnostics.
Bug: https://github.com/dart-lang/sdk/issues/49766
Change-Id: I0c2a2e15f9e1ea92a415bae6e13bbbd373dc613b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255825
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
A get of a promotable field on a non-promotable field shouldn't
promote, otherwise this code would be unsound:
class C {
D get _field1 => D();
}
class D {
final int? _field2 = randomBool() ? 1 : null;
}
main() {
var c = C();
if (c._field1._field2 != null) {
// Problem: `c._field1` returns a different `D` each time, so there's
// no guarantee that `c._field1._field2` is non-null the second time we
// access it!
print(c._field1._field2.isEven);
}
}
This change has no user-visible effect because field promotion is not
yet enabled.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I9a60cd343dff14cded03df700d7c2b62a251487a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255821
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This reverts commit 0715df3d00.
Reason for revert: Could we please revert this temporarily? This breaks copies into G3 (b/243151015). Ivan will be back on Monday and hopefully we will fix it.
Original change's description:
> [deps] rev crypto, file, http, test, web_socket_channel, and webdev
>
> Change-Id: Icc92e4dce84183bac747375b644922845fe908a9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255767
> Reviewed-by: Nate Bosch <nbosch@google.com>
> Commit-Queue: Devon Carew <devoncarew@google.com>
TBR=devoncarew@google.com,nbosch@google.com
Change-Id: I6b14d00ce3227995f8e285a559d8ba9968452961
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255860
Reviewed-by: Devon Carew <devoncarew@google.com>
Reviewed-by: Ilya Yanok <yanok@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
A single internal list pointing to a simple data structure is easier
to reason about than a bunch of parallel lists, and it reduces the
performance cost if we have to add more data to the internal data
structure in the future.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I348cae86781c4d1f15520160a674df381fe2e38f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255815
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Also cleans up and unifies some behavior across the backends:
- avoiding reinstantiating JsInteropChecks for every library
- having the native classes as a member instead of recomputing on dart2js
Now that these checks exist, we can add lib/js/static_interop_test to
the test directories the wasm trybot runs.
Change-Id: I912aae988afe7915e80cc13d00b8c47818dfa520
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255760
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Add YAML metadata to the general issue creation template, so
it shows up in the list of issue creation options for users
on the GitHub issue creation page.
Change-Id: I72f546c1dfd407e41ad75e9abcf080cb5c9ea5ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255661
Reviewed-by: Kevin Chisholm <kevinjchisholm@google.com>
Reviewed-by: Michael Thomsen <mit@google.com>
Since we are reading data in the context of a different reader, we should make sure that we use that reader's helpers.
Change-Id: Id7fbf3662104d131c1b31bff080a171bd3d38a88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255800
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This uses the `Procedure.signatureType` for checking super access
instead of the types derived from `Procedure.function`. This fixes
the checking of super access in face semi-stubs where the parameter
types have a more restrictive but covariant type.
Change-Id: I8b21238687bcec8e5e42a999c5a94d0e65cac28c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255660
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
The test used an unnamed extension effectively making it private,
meaning that the fuzz test that splits the file up doesn't work.
There seem to be no reason for it to be unnamed so this CL names it,
thus fixing the failure when fuzzing.
Change-Id: Iaeb01739dad50f1894a619fe18b81408fa646ce3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255811
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This adds support for the newly added `extern.externalize` and
`extern.internalize` instructions and uses these for converting to/from
`externref` instead of a JS round-trip.
Change-Id: If18d8f44ddf013d4c26bf1597be91bcd0db41c5a
Cq-Include-Trybots: luci.dart.try:dart2wasm-linux-x64-d8-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255462
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
This version adds support for the `extern.externalize` and
`external.internalize` Wasm instructions.
Change-Id: I2285217feff0afa8107ec6685eeec91487667254
Cq-Include-Trybots: luci.dart.try:dart2wasm-linux-x64-d8-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255461
Reviewed-by: Stephen Adams <sra@google.com>
The API for the JS Promise integration API has changed to be part of
the `new WebAssembly.Function` constructor. This requires parameter
types to be specified explicitly. Apparently, these types currently
must be all `externref`.
Change-Id: Ifae634f65cfa377441be4b2dc0643c34b702fbe7
Cq-Include-Trybots: luci.dart.try:dart2wasm-linux-x64-d8-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255061
Reviewed-by: Stephen Adams <sra@google.com>
