The correction producer implements a fix. Pending discussion we might
choose to extend it to produce an assist, either in addition to the fix
(if we keep the lint) or instead of a fix (if we drop the lint).
Whatever we choose, I'd like to do that in a separate CL because I want
this to be available for testing purposes as soon as possible.
Change-Id: Icdd17ba0130149fc785c196be69e9ef990529800
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503520
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
I'm honestly unsure why this is not reported by my IDE, or CI. But it
is reported internally in Google. I'm sure it has to do with the SDK
starting to add some extension methods in the core libs, which were
previously only available in package:collection. But this is still safe,
as analyzer's minimum SDK is Dart 3.11.0.
Change-Id: Ibbb0078e463df03b31749296e1d2f474afb183ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503582
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Fixes https://github.com/dart-lang/sdk/issues/62649
This change overhauls what the PluginServer does when it receives requests from DAS like watch events, overlay changes, setting context roots, and setting priority files, to use the AnalysisDriver APIs. These APIs provide a much smarter, more fine-grained mechanism for determining which files need to be re-analyzed.
This requires a lot of changes that really don't seem obvious (or just,
to me, they are not intuitive), because they relate to Streams being
processed, drained, read, etc. So I'll summarize as much as I can:
* In order to track that results coming from AnalysisDriver are
definitely for the same AnalysisSession we are concerned with, at
any given time, we add `_filesBeingAnalyzed` and
`_filesBeingResolved`. These two maps are cleared any time the plugin
isolate is notified of changes, so there is not a memory concern.
* We add a `waitForIdle` method which is just used by tests
occasionally, in order to ensure analysis is complete.
* We tackle a related bug mentioned in the issue thread regarding
adding or removing overlays without changing the source contents.
There are now a few checks that old-content is not equal to
new-content.
* Due to the more async nature of reading events off of the analysis
driver, we have to be more particular in tests about sending
`AnalysisSetContextRootsParams` and reading from the notification
stream, so there are a lot of changes in tests to re-position these
calls.
This results in massive savings in re-analysis time. Using the example in https://github.com/dart-lang/sdk/issues/62649, typing in the
`build_runner/lib/src/logging/build_log_logger.dart` file, I see the
following:
* Before this fix, every keystroke results in 183 files being
re-analyzed by the plugin (always 183).
* After this fix:
* keystrokes that do not result in a summary change (or maybe
fine-grained deps analysis), like whitespace, results in 1 file
being re-analyzed (the file I'm editing).
* A keystroke that results in slightly changed summary, like changing
an import to something invalid, results 2 or 4 files being
re-analyzed.
* A keystroke that changes the name of class BuildLogLogger (large
change to summary) results in 9-10 files being re-analyzed.
Change-Id: Ie35053b0d90457b5b4b53bc8803188def0308d2a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503040
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This file is no longer published and has no other value. Removing it
removes one step from the process of writing a new lint, which improves
our velocity.
Change-Id: Ie81b914bfa59aa8ec2c8fb2921eec2e9963bd601
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503620
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The session log normalizer replaces known paths/URIs in JSON but doesn't take into account different URI encoding between the client and the server. For example VS Code will encode ampersands whereas Dart does not:
```
file:///c:/uri&encoding&quirks
file:///c:/uri%26encoding%26quirks
```
This means not all file URIs are correctly normalized.
Adding additional groups for each potentially-encoded characters make the regex many times slower (the benchmark test here goes from around 25ms to over 1s per iteration), so instead this change has the normalizer accept the original JSON map and uses jsonEncode()s `toEncodable` option to normalize any URIs (by converting them to their file paths and then encoding using Dart's Uri class) so they will always be consistent before the replacement.
(I tried doing the replacement also in `toEncodable`, but invoking the regex many times also slowed things down a lot).
There is a small time increase (2-3ms) for a payload of 2MB. The "before" times quoted here are slightly higher than previously quoted, but that's because `jsonEncode()` was previously done inside `SessionLoggerFileSink` (and therefore excluded from the timings before), but is now done inside the normalizer to allow normalizing the URI escaping.
Replacing 250 paths in payload of 2097152 bytes
Iteration #1, First: 57ms, Rest: 40ms
Iteration #2, First: 49ms, Rest: 41ms
Iteration #3, First: 45ms, Rest: 41ms
Iteration #4, First: 47ms, Rest: 40ms
Iteration #5, First: 40ms, Rest: 40ms
Replacing 250 paths in payload of 2097152 bytes
Iteration #1, First: 59ms, Rest: 43ms
Iteration #2, First: 53ms, Rest: 44ms
Iteration #3, First: 52ms, Rest: 44ms
Iteration #4, First: 49ms, Rest: 43ms
Iteration #5, First: 49ms, Rest: 43ms
Fixes https://github.com/dart-lang/sdk/issues/63331
Change-Id: Ice2dc7ceceaa6c08e2ff634d7564efe9f0f7de44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502940
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
This updates the IOHandle type in eventhandler_fuchsia to use
zx_port_cancel_key to cancel outstanding asynchronous waits instead of
the (older) zx_port_cancel. The cancel_key operation requires only the
key used to register the initial wait and not the originating handle.
This means that the IOHandle operation no longer has to store the
original handle or worry about its lifetime.
Tested: Patched locally in a test embedder using this logic
Change-Id: Iec0ec632a7dd438a34e482a12b5b298be56ce476
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501840
Reviewed-by: Zijie He <zijiehe@google.com>
Auto-Submit: James Robinson <jamesr@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
At this time, this flag is mainly going to be used to enable partial
testing of the JIT runtime when loading applications from kernel
snapshots.
Bug: b/448095881
TEST=none yet, will be added in upcoming CL
Change-Id: I272d43cdab0c9a9c3653cccc9e96961c1122bbd7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503460
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
This change introduces flags to allow the use of dynamic calls now that
they have their corresponding validation and runtime check. This will
enable us to add end-to-end tests next.
TEST=none yet - will be added in subsequent CL (see CL chain)
Bug: b/448095881
Change-Id: I82448824e94dd940ce5346f79554d34847ecb610
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498320
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Unlike other calls from Dynamic Modules, dynamic calls cannot be
validated entirely at compile time. While we check that the selector
used matches a selector that was allowed (either because a method with
that selector name was exposed as dynamically callable or because the
selector was allowlisted during bytecode compilation), the compiler
doesn't know statically whether the target of the call is exposed.
In prior changes we modified the annotator to add a pragma indicating
whether a member is dynamically-callable or
implicitly-dynamically-callable. Here we use that information to set a
bit on functions and their corresponding dynamic invocation forwarders,
which is verified by the interpreter to make sure the dynamic call is
still allowed.
TEST=none yet - will be added in subsequent CL (see CL chain)
Bug: b/448095881
Change-Id: I27acb4e690a68e08fe1f1ca94e0d77cc7dc4d11e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498300
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Adds isSystemIsolate in the isolate startup message sent to the
VM service, allowing for the service to identify the first non-system
isolate as the root isolate.
TEST=Existing tests
CoreLibraryReviewExempt: dart:vmservice is internal.
Change-Id: I0a982a1fc06bd0be0426ad9d1401e89375cbed40
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500000
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
This replaces the creation of many AST nodes with calls to either the internal or external helpers, to help enforce passing of needed arguments, in particular file offset which are generally not required.
Change-Id: Id9b2449f398492dd5fb8bf0c71fbe3ec04444c95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/497842
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Similar to [0] which recognizes functions that return always nulls, we
recognize functions that never return and
* make the wasm function have no outputs
* make call sites emit `unreachable()` after the call (to inform
binaryen & wasm runtime that this is unreachable)
Before we had an artificial construct where we made such functions
have a `w.RefType(HeapType.none, nullable: false)` return type (i.e.
bottom, i.e. no values) and encoded that way it's unreachable.
We also change some exported wasm functions to explicitly use wasm
types in the signature (namely `WasmVoid` instead of Dart `void`).
[0] https://dart-review.googlesource.com/c/sdk/+/497620
Change-Id: I3724e777cda23c0cf2c8a7dd2e473f3fef0a4f54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499240
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
When there's no expression on the stack but we expect something on the
stack, then the code should be unreachable.
Though the current code would just synthesize a value that matches
the expected type (`convertType(voidMarker, <some type>)`). This
is problematic: If we ever used that synthesized value we may
have incorrect program behavior.
Now there were some valid uses where we synthesize values
* A function that has `void` return type but no explicit return
=> Here we should synthesize `null`
* Synthesize `null` in cases where we know it's not going to be used
=> e.g. for CFE desugaring of `a[i] = b` is roughly
`let tmp = b in (let ignored = a.[]=(tmp) in b)`
where we synthesize `null` as `a.[]=(tmp)` result,
`ignored` isn't used.
* ...
With this CL we no longer allow synthesizing a value of a type
out of thin air, instead all the places where this occurs have
to do that explicitly.
There's some impurities around how setters and index setters
are handled today (and even after this CL). Those impurities
start all the way at CFE, which treats setters and index
setters very differently. See the CFE issue [0].
For those we have two choices:
* special case all call sites that require synthesizing
null values
* special case all call sites that require dropping an
auto synthesized null value
This CL now marks instance setter/index-setter methods as
requiring auto-synthesizeing null values on usage sites and
make code that doesn't need them explicitly drop them.
Somewhat related to this change is how we deal with `void`
on the Dart <-> Wasm Import / Wasm Export boundary: When we
call an imported wasm function that has `void` as return
type (meaning no return values) we have to synthesize a `null`
(as the caller may "use"/"observe" the `void`).
=> We now are more strict and instead use `WasmVoid` as type
instead of allowing `void` as type on the import/export
functions.
[0] https://github.com/dart-lang/sdk/issues/63360
Change-Id: Ie30df3bd68553724437607bab3163c98f5467efe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501061
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
https://dart-review.googlesource.com/c/sdk/+/498840 added support for
promotion of properties (private, final instance variables with a name
which isn't used much for other purposes) in the context of anonymous
methods.
This CL reduces the set of situations where this feature is enabled such
that only `this` will allow property promotions to be carried in (such
that `this._x` is promoted in `v.=> this._x` when `v` is such that
`v._x` has been promoted before the anonymous method occurs). It also
generalizes the mechanism such that property promotions are carried out
(so we can do `if (v.=> _x is int) v._x.isEven;`).
Tests has been adjusted accordingly.
Change-Id: Ibe70713d3d9c89a6d95f9c3dd28df8f147cb518d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502660
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
When deferred loading was enabled the dynamic dispatch class ID table
was incorrectly assigning class IDs to the wrong module. If a contiguous
target segment included classes/targets from different modules, all of
them were getting assigned to the module of the first class/target in
that segment.
This was causing spurious NSM exceptions as the necessary rows in the
table might not be populated for a dynamic call if the module the
segment was assigned to wasn't loaded yet.
To fix this we end the segment if the next target does not belong to the
same module as the active segment.
The new test fails with an NSM exception prior to this fix.
Change-Id: I07bc4fdb5a8bff1bfad5fe17f45c8076a965a775
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502860
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Brian Wilkerson