This reverts commit bbacf39e9c.
Reason for revert: Causing flutter/engine to fail to roll into flutter/flutter. See https://github.com/flutter/flutter/pull/125363.
Original change's description:
> [analysis_server] Analyze fix data in fix_data folder
>
> Fixes part of https://github.com/dart-lang/sdk/issues/52126.
>
> Change-Id: Ib4bd7830a2f644eacedccd375c7c8dc60f040d33
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296801
> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I109a4b2c596ad22d73eaf0ac3e25f53a35ba5e28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297320
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Zach Anderson <zra@google.com>
This CL replaces https://dart-review.googlesource.com/c/sdk/+/296900
The `List` constructor is removed in Dart 3.0.
Some of the `@patch` implementations were not removed.
This is *high priority*. It seems the left-over `@patch factory List` constructor did not cause any errors, instead it *added* a constructor to `List` that can be used in web compiled code. Even if `List` doesn't have such a constructor in the SDK code proper.
The VM and analyzer will say the invocation is an error, but dart2js happily compiles it and runs.
(It used to be that patches couldn't add public members, that security seems to have been removed.)
Also removes code which tries to detect "the unnamed List constructor",
which is no longer a thing, and a number of invocations of the constructor, where it's not clear that the test is aware that the constructor no longer exists, and is not marked as `@dart=2.x` with x < 12.
TEST=ci
Change-Id: I4ffaf3ae2c4e75ca06e7ba0bf19187b6376f3888
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297100
Reviewed-by: Brian Quinlan <bquinlan@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
What was being logged as "kernel bytes" or "Dart characters" included the size of the platform dill as well. Changed the text for that to the more general "input bytes". The new source bytes number is the number of bytes registered to Dart2JS by the CFE. This registration is done when the CFE is being run modularly or directly from sources.
When being run modularly the input to phase 0 (CFE-only) is slightly larger (includes extra files) but the input to phase 1 (closed world) ends up matching the non-modular value.
Example output:
Compiled 14,927,936 input bytes (8,010,096 characters source) to 11,647,944 kernel bytes in 0.48 seconds using 114.082 MB of memory
Change-Id: Idaaaf6f9c906659434a9af50882b7ed3343e601a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297020
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Before the calling convention change during the introduction of AOT, this was used to get code metadata associated with a frame. Its remaining non-test use is forming service IDs for code object, which can use the regular visitors.
TEST=ci
Change-Id: Ica8eafe988d19eba8d905d9dc5907afbfd559118
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296705
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
After the Page is unregistered, the only reference to its VirtualMemory is in memory not traced by LSAN. If exit happens here before the VirtualMemory is deleted or put into the global cache, it appears to be leaked.
TEST=lsan
Bug: https://github.com/dart-lang/sdk/issues/51560
Change-Id: I973e09bb19910d9cdcc3af6c8c9a188f095df36a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297040
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
The analyzer team has decided to adopt the convention of using `TODO`
comments to document long term issues that should persist in the
codebase, and `FIXME` comments to document short term issues that need
immediate attention. They may even consider adding a presubmit hook
to ensure that `FIXME` comments are only used during local
development.
Accordingly, it makes sense to suppress `TODO` comments from being
surfaced to the IDE "problems" view (since there are hundreds of them,
and they're not immediately actionable). This makes VSCode's
"problems" view much more usable in "tree" mode.
Change-Id: I11a0c59132fb98c1c86fb4adf22d1fdf3b547c80
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295662
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
These dependency overrides prevent pub from getting confused about the
fact that our SDK development is in a mono-repo; that in turn will
allow unit tests to be run from inside vscode using the standard test
integration (see https://github.com/Dart-Code/Dart-Code/issues/4502).
Change-Id: I8ea709984b03927e67f119d147338203d7e80811
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296980
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The official dart-sdk packages are now automatically built in the CI and
developers will no longer need to build them manually when rolling the
checked in SDK. The cipd instances are now tagged with the commit hash
instead of the version number, which allows us multiple attempts at
building a particular version (which different commits).
Bug: b/279024347
Change-Id: I4bfb4a5384bb8c671b576afdac0e5008ccfcc8a1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296861
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Jonas Termansen <sortie@google.com>
The current hot-reload implementation [0] will perform a reload by
first sending OOB messages to all isolates and waiting until those OOB
messages are being handled. The handler of the OOB message will block
the thread (and unschedule isolate) and notify the thread performing
reload it's ready.
This requires that all isolates within a group can actually run & block.
This is the case for the VM implementation of isolates (as they are
run an unlimited size thread pool).
Though flutter seems to multiplex several engine isolates on the same OS
thread. Reloading can then result in one engine isolate performing
reload waiting for another to act on the OOB message (which it will not
do as it's multiplexed on the same thread as the former).
Now that we have a more flexible safepointing mechanism (introduced in
[1]) we can utilize for hot reloading by introducing a new "reloading"
safepoint level.
Reload safepoints
-----------------------
We introduce a new safepoint level (SafepointLevel::kGCAndDeoptAndReload).
Being at a "reload safepoint" implies being at a "deopt safepoint"
which implies being at a "gc safepoint".
Code has to explicitly opt-into making safepoint checks participate /
check into "reload safepoints" using [ReloadParticipationScope]. We do
that at certain well-defined places where reload is possible (e.g. event
loop boundaries, descheduling of isolates, OOM message processing, ...).
While running under [NoReloadScope] we disable checking into "reload
safepoints".
Initiator of hot-reload
-----------------------
When a mutator initiates a reload operation (e.g. as part of a
`ReloadSources` `vm-service` API call) it will use a
[ReloadSafepointOperationScope] to get all other mutators to a
safepoint.
For mutators that aren't already at a "reload safepoint", we'll
notify them via an OOB message (instead of scheduling kVMInterrupt).
While waiting for all mutators to check into a "reload safepoint", the
thread is itself at a safepoint (as other mutators may perform lower
level safepoint operations - e.g. GC, Deopt, ...)
Once all mutators are at a "reload safepoint" the thread will take
ownership of all safepoint levels.
Other mutators
-----------------------
Mutators can be at a "reload safepoint" already (e.g. isolate is not
scheduled). If they try to exit safepoint they will block until the
reload operation is finished.
Mutators that are not at a "reload safepoint" (e.g. executing Dart or VM
code) will be sent an OOB message indicating it should check into a
"reload safepoint". We assume mutators make progress until they can
process OOB message.
Mutators may run under a [NoReloadScope] when handling the OOM message.
In that case they will not check into the "reload safepoint" and simply
ignore the message. To ensure the thread will eventually check-in,
we'll make the destructor of [~NoReloadScope] check & send itself a new OOB
message indicating reload should happen. Eventually getting the mutator
to process the OOM message (which is a well-defined place where we can
check into the reload safepoint).
Non-isolate mutators such as the background compiler do not react to OOB
messages. This means that either those mutators have to be stopped (e.g.
bg compiler) before initiating a reload safepoint operation, the
threads have to explicitly opt-into participating in reload safepoints
or the threads have to deschedule themselves eventually.
Misc
----
Owning a reload safepoint operation implies also owning the deopt &
gc safepoint operation. Yet some code would like to ensure it actually
runs under a [DeoptSafepointOperatoinScope]/[GCSafepointOperationScope].
=> The `Thread::OwnsGCSafepoint()` handles that.
While performing hot-reload we may exercise common code (e.g. kernel
loader, ...) that acquires safepoint locks. Normally it's disallows to
acquire safepoint locks while holding a safepoint operation (since
mutators may be stopped at places where they hold locks, creating
deadlock scenarios).
=> We explicitly opt code into participating in reload safepointing
requests. Those well-defined places aren't holding safepoint locks.
=> The `Thread::CanAcquireSafepointLocks()` will return `true` despite
owning a reload operation. (But if one also holds deopt/gc safepoint
operation it will return false)
Example where this matters: As part of hot-reload, we load kernel which
may create new symbols. The symbol creation code may acquire the symbol
lock and `InsertNewOrGet()` a symbol. This is safe as other mutators
don't hold the symbol lock at reload safepoints. The same cannot be said
for Deopt/GC safepoint operations - as they can interrupt code at many
more places where there's no guarantee that no locks are held.
[0] https://dart-review.googlesource.com/c/sdk/+/187461
[1] https://dart-review.googlesource.com/c/sdk/+/196927
Issue https://github.com/flutter/flutter/issues/124546
TEST=Newly added Reload_* tests.
Change-Id: I6842d7d2b284d043cc047fd702b7c5c7dd1fa3c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296183
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
The sweeper threads are bypassing safepoints and have no need for normal
mutator state (e.g. storebuffer, marking stacks, .,..) on the [Thread]
object.
This fixes a TSAN report where sweeper would clear reusable handles
(which it didn't actually modify) and scavenger is reading those
handles (which arguably it doesn't have to either).
Issue https://github.com/dart-lang/sdk/issues/52125
TEST=ci
Change-Id: I03a36e8518b6c00eb7f3b57f65fd469dddba23e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296860
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
An embedder (or the VM) can exit an isolate via `Thread::ExitIsolate()`
at a point where there's still active state (e.g. dart frames).
Because of this the VM has so far conservatively retained the [Thread]
object of dart mutators throughout the isolate's lifetime. After which
is was manually `delete`ed. We'd never re-use those [Thread] objects (we
do re-use [Thread] objects of non-dart-mutator threads).
When exiting via `Thread::ExitIsolate()` with active state, the mutator
was assumed to be at-safepoint at all levels. It was removed from the
thread registry's active threads. This also means that when e.g. GC runs
it can't use the thread registry to find all active threads it may
need to scan, instead it uses [Isolate::mutator_thread_] of all isolates.
This causes a variety of subtle issues, but the main one that motivated
this change is the following:
If a thread obtains a safepoint operation it means all other mutators
are parked. The thread owning the safepoint can do whatever it likes.
When introducing reload operation safepoints, a thread may want to
ReloadSafepointOperation reload(thread);
...
// Compile sources.
{
TransitionVMToNative transition(thread);
// Will temporarily exit & re-enter current isolate.
response_port = Dart_NewNativePort();
Dart_PostCObject(kernel_isolate_port, ...);
// Wait on [response_port] for response.
}
This will cause the reloading thread to own the reload safepoint
operation but still transition states and even exit/re-enter the
isolate. Though this is currently not possible in the way enter/exit is
implemented.
So we'll refactor this fragile code in the following way:
* Move thread enter/exit logic entirely to the [Thread] object.
* Keep used threads in the thread registry's active list.
=> This allows us to keep various state on the [Thread] and thereby
avoids clearing it when suspending & re-initialing it when resuming
=> It makes nested `Thread::ExitIsolate()` faster as we mainly have
to enter safepoint (avoid acquiring threads lock, avoid releasing
storebuffers, ...)
=> It makes nested `Thread::EnterIsolate()` faster as we mainly have
to leave the safepoint (avoid acquiring threads lock, avoid acquiring
storebuffers, ...).
=> A mutator can now own a safepoint operation (e.g. reload safepoint
operation) and still `ExitSafepoint()` / `EnterSafepoint()` safely -
as those are based on the normal `EnterSafepoint()` and
`LeaveSafepoint()` APIs.
* We separate
- Suspend & Resume of a dart mutator (possibly with active stack)
- Setup & Reset of state only relevant for dart mutators
- Setup & Reset of state relevant for any mutator
* We unify how the [Thread] objects are freed between dart mutator and
non-dart mutators: [Thread] objects without state can be given back to
the [ThreadRegistry] and re-used (instead of being deleted in
`Isolate::~Isolate`)
* We have capability to free [Thread] objects if a dart mutator has an
empty stack & re-use for another isolate of the same group.
(In future we may have N Thread objects for N cores and the threads
would even maintain their TLABs when switching between isolates)
* Since we allow reusing of [Thread] objects also for dart mutators now,
we have extensive asserts to ensure they are "clean" when they get
into the free list and come out "clean" again.
TEST=ci
Change-Id: Id85e8e484efd98d28e323b33795716420e619986
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296585
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Multitests aren't valid Dart files that can be processed by a Dart
implementation so the updater generally does a poor job if it tries to
update one.
It's probably not worth supporting because, in practice, a test should
either be a multitest or a static error test, but not both.
Change the tool to skip over any multitests it encounters. If this
results in it doing nothing at all, it reports that as an error.
Otherwise, it just lists the multitests it didn't process.
Close#37721.
Change-Id: Icfb1ff9fe63f2c249b3ccfba65166b97654a9918
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296760
Commit-Queue: Leaf Petersen <leafp@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
This makes the socket benchmark (and real world code also), about 100x faster.
Change-Id: Iaa130b7bf69e4c89d3c3a221680e5b62ffe10583
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296641
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
In order for an analyzer quick fix to aid the user in correcting
exhaustiveness errors, it needs to know not just the text of the
witness pattern that needs to be inserted, but also which elements are
referenced by the identifiers in that pattern. This will allow it to
add in imports if the user's code doesn't yet import the necessary
library, or insert import prefixes in the case where the user is
already importing the necessary library in a prefixed fashion.
To reduce the effect on the exhaustiveness logic (which was designed
with the intention of building up the witness pattern into a
StringBuffer), we introduce a new type, `DartTemplateBuffer`, which
behaves similarly to StringBuffer but also captures the necessary
semantic information so that the analyzer will be able to fix up
imports in an appropriate way.
This change is mostly isolated to the _fe_analyzer_shared package; it
doesn't add the necessary logic to the analyzer to capture the
elements; it simply plumbs them up to the API boundary between the
packages. Future CLs will add the necessary analyzer logic to make
use of the information.
Change-Id: I7c030fc49a2510acbefd6aeb567e0e7b7102855a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296385
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I3a41bb83767052abda1fdfc77e21bd1a83188482
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296441
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I35d4a7c7ad09cb77ff379af036180794d8b6afbf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296403
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: Idf9012cb8c213b523d1c8bb827e530e0d2cf6609
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296407
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The latter will factor into a page's alignment offset.
TEST=ci
Change-Id: Iee5117efb3278399e0fd09b21ea92eadc8237296
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263680
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I4c2840deffe5d790a22facebbcc8a02c1cb98020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296425
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
State more explicitly that sealed classes are abstract and lead the
user to a better fix with the error message.
Bug: https://github.com/dart-lang/sdk/issues/52073
Change-Id: Id24c6cb187ee5497ca2819f930c48ff5aa8d07fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296025
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Change-Id: I7192ba0fe31f2059c722559c4acaec4bdf0b01f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296022
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I3e10209e78e48893d2f2df7f8af7963d319efd9f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296405
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I6218c6ea339b6c19c1495b1db9b2da3fe1654718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296440
Commit-Queue: Jake Macdonald <jakemac@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
