While working on some other changes to flow analysis, I discovered
that this particular behaviour wasn't unit tested.
Change-Id: Ia9b27672c62177ffed80d4143f33c5b764ac7bbe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313242
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
My previous CL changed too much... This test specifically didn't want a valid absolute path on Windows, it wanted one without the drive letter.
Change-Id: If0812517e506f9960bca7ac99dd713be087b9ad9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/314020
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
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>
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>
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>
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>
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>
The Memory column is right aligned, and we can have large memory values.
`'| Memory | CPU | Elapsed time | ...`
Closes: https://github.com/dart-lang/sdk/issues/52915
Change-Id: I47c836ca85109bfa99e293c5c955c416c0f4067d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313382
Reviewed-by: Martin Kustermann <kustermann@google.com>
Auto-Submit: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
A user found another case where this fails (b/290868787).
In this case a ForwardingTypeMask is delegating the subtyping checks to its `forwardTo` mask which ends up saying the masks are equal even though the wrapper around the forwarded masks means they are not equal.
Change-Id: I015f2faf57557ff4189acb6c7144d09cf1ae6bb6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313341
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
The mixin `LspAnalysisServerTestMixin` contains a lot of helpers for interacting with the LSP server in tests and is used by both the in-process tests and the out-of-process integration tests.
We now have a third category of LSP tests that would benefit from many of these helpers (such as "getHover()") but are not talking to a native LSP server.
This change splits those helpers out into `LspRequestHelpersMixin` which contains only the request helpers without any code assuming a native LSP server, so they can be used by LSP-over-Legacy tests.
All code in `request_helpers_mixin.dart` was lifted directly from `server_abstract.dart` with no changes.
It also adds a test that error responses in LSP-over-Legacy are handled appropriately.
Change-Id: I847855a5314b5b04d700c6400b67e76e5f0d3402
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313362
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This adds two custom requests to the DAP-over-DDS handler to translate between VM/DAP instance IDs:
- $/createVariableForInstance (String isolateId, String instanceId)
- $/getVariablesInstanceId (int variablesReference)
Because DAP's variables request only fetches _child_ variables (eg. fields) but we'd likely want to align the top-level string display of a variable, the wrapped variable will first be returned as a single variable named "value", which contains both the string display value and also a variablesReference to then get the child variables (usually invoked when expanded).
These methods currently live directly in the DDS DAP adapter since I figure they're only useful for clients using the VM Service, but we can move them to the base adapter if in future this turns out to not be the case.
Change-Id: I60f28edd86c3468cc592175cb665557a1fc85056
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312987
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
The assertion in the `_FlowAnalysisImpl` constructor was unnecessary
because it was checking that all variables that are reported to
`AssignedVariables` as read or written must also be reported as
declared. This is already checked by assertions in
`AssignedVariables.finish`, which is called by the `_FlowAnalysisImpl`
constructor.
I've added tests to `assigned_variables_test.dart` to confirm that
these assertions work, and I've also cleaned up the assertions a bit,
eliminating some redundancy and ensuring that in the event of a
failure the assertion failure message will be comprehensible.
Change-Id: Ife827c91d944707f093f4cb8421385f5355d11fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313140
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
These become a single property of ExecutableElement, MethodElement.
No new visitors, no new nodes and elements.
Change-Id: Ic523e9d51356e5a8d2a6cd62cd508344f561ffb0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312960
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This method caused invalid exhaustiveness checking by seeing
record types as related based solely on the structure.
The original purpose of the methods has been removed in the mean time
by the change to restrict the created spaces by the matched value
type.
Closes#52800
Change-Id: I8fb581374a4813dc63261d9e1354c4eea94f212c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312982
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Right now `utf8.encode()` has a static return type of `List<int>`
due to extending `Encoding` (which extends `Codec<String, List<int>>`).
We cannot easily change `Encoding` to extend `Codec<String, Uint8List>`
because that would also change `utf8.decode()` to require `Uint8List`
which would be a breaking change.
So instead we override `utf8.encode()` to have more precise return type.
Some parts of our SDK are run using the checked-in SDK, so it cannot
rely on the changed return type yet (until checked-in SDK is rolled).
So we use `const Utf8Encoder().convert()` as a temporary change, as
that already has `Uint8List` return type.
Issue https://github.com/dart-lang/sdk/issues/52801
TEST=ci
CoreLibraryReviewExempt: More precise return type for existing API
Change-Id: I2861d1f0eb3d292d8e3ec8437c0d441a2d2bd193
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254903
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
