This change doesn't seem to have a significant impact on most compilation results:
- Golem results show no significant difference in microbenchmarks.
- For a medium and large app tested, while we see a small change in the actual inference results, the generated code is identical before/after this change.
- Timing and memory usage on internal compilations seem comparable before/after this change.
Note: This replaces the need for any notion of "invalid" refines so I will clean up that code in a follow up change.
Change-Id: I2a293eacd944fc17ee2dab97d3d947c042b4038f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313720
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
When using the "mini-ast" pseudo-language to write unit tests for the
flow analysis and type analysis logic in the `_fe_analyzer_shared`, it
is no longer necessary to use `.stmt` to turn an expression into an
expression statement; this now happens automatically. The way this
works under the hood is that both the `Statement` and `Expression`
classes mix in the `ProtoStatement` mixin; constructs that expect
statements are declared with input parameters of type
`ProtoStatement`, and they automatically convert expressions to
statements when necessary.
Also, the functions `checkNotPromoted`, `checkPromoted`,
`checkReachable`, `localFunction` now have a return type of
`Expression` rather than `Statement`. This allows them to be used
either where an expression is exprected or where a statement is
expected, which should give us the ability to write some tests that
are not possible (or very difficult) to write today.
Change-Id: I9f7ad5b15bcf8ccfccafc6985e0163b550c5ad1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313680
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Minor changes to last few 3.0 diagnostic message changes (late follow up). Mostly just a practice CL for me, after setting my sdk environment up for the first time.
Fixes https://github.com/dart-lang/site-www/issues/4740
Change-Id: I8f6871a270089627538928dd95bcbf38a29b74e7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309780
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Marya Belanger <mbelanger@google.com>
This allows changing the in-process tests to use Windows-style paths on macOS for convenience.
Change-Id: I0e85a4f8e831471925b8308ad348f75b6867a53b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313385
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Add test.py --default-suites that includes all the default suites in
addition to the ones explicitly requested, so the test matrix can run
co19 together with the default suites in one sharded test step.
Bug: b/290617138
Change-Id: I5dd5d1aaf3b1ee38adf88c6e9ee6ec13d97fe1ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313567
Reviewed-by: William Hesse <whesse@google.com>
The non-sharded tests takes 10 minutes and each of the sharded tests
takes 14 minutes. It's faster to shard immediately (costing one bot
more) and concurrently run the local tests.
Fix end to end dart2js test that times out when sharded and run outside
a directory called sdk.
Bug: b/290617138
Change-Id: If71f0d301edf565c9f15847098320106ca383635
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312983
Reviewed-by: William Hesse <whesse@google.com>
The try builder is among the fastest commonly used builders at 21
minutes average, which is less than the current 25 minute best case
goal. Free up 2 of 10 shards to be available for other work during peak
load, which will slow it down by 2 or 3 minutes and finish at the same
time as the other try builders, resulting in an overall faster commit
queue experience.
Bug: b/290617138
Change-Id: Iea2ba71248fa0d3c21f1d80b540b55ce810d96af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313565
Reviewed-by: William Hesse <whesse@google.com>
a) Remove 2 fields in our object representation:
* PatchClass::library_data_
* Library::kernel_data_
=> This saves O(#libraries + #patch-classes) which can amount up to
10+ MB for big apps, as we save not just the slots but the
[ExternalTypedData] objects they used to reference.
Instead we'll compute the KernelLibraryData when we need it by making a
[TypedDataView] of sub parts of the component.
b) We make a kernel binary be represented by a single [ExternalTypedData].
Whenever we need a sub-part of a kernel binary (e.g. one for each
component for concatinated kernels, or one for a library inside a
component) we use proper [TypedDataView]s for that.
c) As we sometimes need to create a view of only a particular library
within a component we need to find start/end of a library based on
index.
=> We store the library index instead of the library offset on
Library/PatchClass.
=> We can easily derive the start/end of a library from it's index by
looking at the kernel component encoding.
d) We make the [Reader] object work purely based on a pointer - instead
of making it have if/else when reading bytes (either from pointer or
from a view).
e) We make the [KernelProgramInfo] store the kernel_component and
various TD views into it.
TEST=ci
Change-Id: Ibe160881ff48635e834c3d647a977a144b5d0565
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313561
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Creation of AppJit snapshot will not include the source of [Script]
objects (see `UntaggedScript::snapshot_to`). As a result there's no
point in letting the CFE embed the sources into the kernel if we
only use it to create an AppJit snapshot.
This is also in line with build rules for our dart-sdk, where we do the
kernel compilation separately, see utils/application_snapshot.gni:
```
template("application_snapshot") {
...
# Build the kernel file using the prebuilt VM to speed up the
# debug and simulator builds.
prebuilt_dart_action(target_name + "_dill") {
...
args = [
...
"--no-embed-sources",
...
]
[[[
}
...
}
```
TEST=ci
Change-Id: I4e17e49dc21af6102d62c2278dbd6ebbe387f7e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313560
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Insert CheckNull instructions if in AOT mode prior to checking the
index for being within the bounds in inlined indexing operations.
If we're in --sound-null-safety mode, they'll be canonicalized away
since the indices won't be nullable anyway.
Fixes https://github.com/dart-lang/sdk/issues/52910.
TEST=corelib{,_2}/list_removeat_test and
co19_2/LibTest/core/String/codeUnitAt_A03_t01 on
vm-kernel-precomp-linux-release-x64-try
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I37212e94c7c5032c75709e3f0bb91734cbc705cd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313507
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This new approach uses static tear-offs where possible to avoid allocating a new Closure for each Deferrable.
Note: This doesn't scale to every case but as of today the one case not covered is a singleton and so only 1 closure should be allocated there regardless.
Data from a fairly large app:
Before:
Total => 3.8GB
Closures => 85.6MB
Closure Context => 71.0MB
After:
Total => 3.7GB
Closures => 28.9MB
Closure Context => 49.8MB
Diff:
Closures => 56.7MB
Closure Context => 21.2MB
Change-Id: If233f958df2822708b51c0d14c7439b5d3a5a07b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313340
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
When tracing closures, we accidentally ignored the fact that returning from a non-async
function allows the value to flow in other ways (e.g. through the completion of a future).
This changes the node tracer to always consider the async marker when looking at values
that flow into 'MemberInformation' information nodes, which are the nodes we use
to represent the returned value of a method.
Fixes#52825
Change-Id: I3322e105dc9612f47a516a17f9465bf1002a9f87
Fixed: 52825
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312708
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Sync compare URI implementation with linter
Change-Id: I23e14459ac55fef4cec1c0c41341fe6f51b781b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313285
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Oleh Prypin <oprypin@google.com>
This reverts commit 16fcfe7eae.
Reason for revert: Flutter changes haven't landed to google3 yet.
Original change's description:
> [dart:js_interop] Remove Object.toJS and JSNumber.toDart
>
> Modifies JSBoxedDartObject reified type on JS backends and also
> modifies JSBoxedDartObject.toDart now that a proper box is introduced.
> Also uses a JS symbol in JSBoxedDartObject.toDart for a property
> check so that different Dart apps don't accidentally share Dart
> objects. It's now an error to call this function on non-boxed objects.
>
> Fixes a type issue where creating a new object literal with the JS
> foreign function was resulting in dart2js thinking toJSBox would
> always throw. Changing the typeDescription to PlainJavaScriptObject
> instead of =Object fixes that issue.
>
> CoreLibraryReviewExempt: Backend-specific library.
> Change-Id: I5cfb1f32ff4328fafdf9831b0d8da806c39391d9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309082
> Reviewed-by: Joshua Litt <joshualitt@google.com>
> Commit-Queue: Srujan Gaddam <srujzs@google.com>
Change-Id: I469ad04db7b49ffef47a46ccac97e909e4865719
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313580
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Modifies JSBoxedDartObject reified type on JS backends and also
modifies JSBoxedDartObject.toDart now that a proper box is introduced.
Also uses a JS symbol in JSBoxedDartObject.toDart for a property
check so that different Dart apps don't accidentally share Dart
objects. It's now an error to call this function on non-boxed objects.
Fixes a type issue where creating a new object literal with the JS
foreign function was resulting in dart2js thinking toJSBox would
always throw. Changing the typeDescription to PlainJavaScriptObject
instead of =Object fixes that issue.
CoreLibraryReviewExempt: Backend-specific library.
Change-Id: I5cfb1f32ff4328fafdf9831b0d8da806c39391d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309082
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
A basic bisection script.
Currently only supports substring matching for detecting the error.
This was enough for three use cases today:
* https://github.com/dart-lang/sdk/issues/52910
* https://github.com/dart-lang/sdk/issues/52911
* https://github.com/dart-lang/sdk/issues/52912
Produces a concise output on standard out, and a very detailed log
with all process invocation results in `.dart_tool/bisect_dart`.
Usage: tools/bisect.dart -Dstart=23f41452 -Dend=2c97bd78 -Dtest_command="tools/test.py --build -n dartk-linux-debug-x64 lib_2/isolate/package_resolve_test" -Dfailure_string="Error: The argument type 'String' can't be assigned to the parameter type 'Uri'." -Dsdk_path=/usr/local/google/home/dacoharkes/dart-sdk/sdk/ -Dname=20230712_package_resolve_test
This script starts a bisection in the provided SDK path.
It will write logs to .dart_tool/bisect_dart/.
start : The commit has at the start of the commit range.
end : The commit has at the end of the commit range.
test_command : The invocation of test.py.
This should include `--build`.
This should be within quotes when passed in terminal because of spaces.
failure_string : A string from the failing output.
Regexes are not yet supported.
This should be within quotes when passed in terminal when containing spaces.
sdk_path : The SDK path is optional.
The SDK path defaults to the current working directory.
name : The name is optional.
The name defaults to the current date and the recognized test name.
The name is used for distinguishing logs.
Change-Id: Ib071a5305d4992cf189e35eb3dcc50c83101503e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313384
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The test steps with the same configurations have been unified and
the shard number balanced so each step finishes as closely to each
other as possible.
Bug: b/290617138
Change-Id: I6616fb78e5cd6db36153db0d9e2b45131eef25fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312984
Reviewed-by: William Hesse <whesse@google.com>
A named optional parameter could previously be equipped with a default
value using ':' or '=' (as in `void f({int i : 1})` or `... i = 1 ...`).
The language has now (Dart 3.0) been updated to eliminate the form
using ':' (it has been deprecated for a long time).
This CL updates the spec_parser accordingly.
Change-Id: I28f9ec605699a9ffb2f7349a6a4b698fa709247b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313482
Commit-Queue: Erik Ernst <eernst@google.com>
Auto-Submit: Erik Ernst <eernst@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: William Hesse <whesse@google.com>
When running with `--no-sound-null-safety` turned on, it turns out null
is unboxed as an Mint in some cases without checking for null first.
Before, tests would fail because unboxing it would give a really
large int that was unlikely to be acceptable to subsequent range
checks and the like.
However, since 2f63ace, that memory is now zero-initialized, and zero is
more likely to be an acceptable value, so tests either fail for
unexpected reasons or, worse, unexpectedly succeed.
As a stopgap until the appropriate checks are emitted, we initialize
the contents of null with its address as an ObjectPtr like we used to.
TEST=corelib{,_2}/list_removeat_test on dartkp-* configurations.
Issue: https://github.com/dart-lang/sdk/issues/52910
Change-Id: If456d503c86202616f4f566a402118e9c41194ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313500
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
set_pointer_offsets_length() sets a part of the state_bits bitfield,
so we need to move this check before it's called.
Fixes https://github.com/dart-lang/sdk/issues/52923
TEST=vm-linux-debug-ia32 builds
Cq-Include-Trybots: luci.dart.try:vm-linux-debug-ia32-try
Change-Id: I9f4ec78b03a6b7b2d45f13e9cc277411dcc90493
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313480
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
** KernelProgramInfo
The VM can load multiple kernel files into an isolate. Each kernel file
is represented as a [KernelProgramInfo] object. Loading a kernel blob
will result in a tree of objects
Library
- TopLevelClass
+ Field
+ Function
+ Class
+ Field
+ Function
+ ... closure functions ...
The entire tree belongs to one [KernelProgramInfo] object. There are
two exceptions
* after a hot-reload we inject [PatchClass]es (as owners of
old [Field]/[Function] objects) that identify the old kernel data
* expression evaluation functions will get a special data array with
information about the expression evaluation kernel data
=> We attach the [KernelProgramInfo] to the root of the tree, namely
the [Library]
=> We introduce `{Field,Function,Class,Library}::KernelProgramInfo()`
that will get that object - by walking up the chain.
=> We introduce `UntaggedPatchClass::kernel_program_info_` field that
allows saving the old [KernelProgramInfo] (on which the old
[Field]/[Function] objects are based upon) in the [PatchClass].
=> We include the [KernelProgramInfo] in the expression evaluation
data array.
** Scripts
A [Script] object should represent a .dart source file. It contains the
source, line offsets, etc. of that dart source file. The contents of a
dart source file may be used by many classes/libraries or even many
kernel blobs (e.g. via our copying mixin application).
When we build a tree like above, we'll attach a reference to a
[Script] object to [Class] objects. But members of a class may come from
a different script, in which case we inject an intermediary [PatchClass]
that references that other script.
When the compiler frontend loads kernel, builds flow graph, etc we walk
down the kernel binary (the tree above) - all belonging to the same
[KernelProgramInfo] object, but sub-parts may belong to different
[Script]s.
=> We use the existing [ActiveClass] class that keeps track of the
current class & member inside the class we're operating on.
=> Whenever we need the script we get it from the active member
function/class.
** Misc
We make kernel related classes (e.g. [KernelReaderHelper]) independent
of [Script].
We make the [Script] objects hold on to the [KernelProgramInfo] they are
based upon (e.g. for lazy source reading). But since a given [Script]
doesn't belong to a particular kernel blob (multiple kernel blobs may
contain information about the same script) we don't expose this in the
public API.
=> In the future we could have a unique [Script] object per url (instead
of having several as we do now)
TEST=ci
Change-Id: I7bd698f497b0bd8ab8e546540bb1dad5b5a6dd48
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313381
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This reverts commit fb9d0e6dc3.
Reason for revert: Performance regression https://github.com/dart-lang/sdk/issues/52922
Original change's description:
> Tweaks for mixin inference.
>
> I started initially with the idea to move 'mixinInferenceCallback'
> to `ClassElementImpl`, but then realized that we do it for enums too.
> So, it should stay in `InterfaceElementImpl`.
>
> I left with code that I think slightly modernized, so decided to
> send it for review.
>
> Change-Id: Ib990392dc4985a71ffba1f4080237872d9a65ad2
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312521
> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I33098d8509319e9d6e62d7f5174cae2c05977436
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313440
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Jaime Wren <jwren@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Change-Id: I37070079ddbdc86fb0e5144842690877abb1b031
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313283
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Added helpers `_field` and `_localVar` to allow tests to test their final fields and local const variables in the const tests.
The rest of the CL moves away from `_evaluateConstant` helpers (to avoid recomputing constants), to `_topLevelVar` or equivalent helpers which grabs the existing evaluation result.
This CL adds to the goal making all constant tests consistent and to avoid unnecessary const computations in the tests.
Change-Id: I508483714a51e5d060286256657ae460b65787c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312889
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
