Before this change, an error reaching an FFI callback would attempt to execute the normal invocation stub from the beginning in the FFI callback's frame, which quickly crashes. After this change, the runtime recognizes this marker use of the invocation stub and returns to the FFI callback function instead.
TEST=ffi/unwind
Bug: https://github.com/dart-lang/sdk/issues/39487
Change-Id: I477cfcfc236e6cf518ebfe52860ba49e466ebf8b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409562
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
We start isolates paused so that we can send breakpoints before any code runs. This means we need to resume after initialization is complete.
It's important we don't try to resume multiple times during initialization (regardless of the order of isolate events or whether the isolate was found when we queried for isolates during connection).. This is done with the flag `startupHandled`.
One code path was not taking this flag into account, which meant multiple resumes were still possible. This seemed to occur on Linux during test runs (I've noticed the order of events being different on Linux in the past).
This change extracts the checking of `startupHandled` before sending `readyToResume` and uses it in all places that handle this kind of startup resume.
Fixes https://github.com/dart-lang/sdk/issues/60128
Change-Id: Ie2679fc806ab3edf007259298da82dbc8b802a6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410760
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Sometimes json strings contain escape characters. If they do, then a
slower path function is triggered. This CL makes that slower part also
take advantage of ASCII property (in a similar way as we do if the
entire json string is ASCII).
CoreLibraryReviewExempt: Changes only non user visible internals.
Change-Id: Ib055a579778b5ba58a9680da1b34624b095af22f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410501
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This change uses Gerrit OWNERS file syntax (documented here:
https://gerrit.googlesource.com/plugins/find-owners/+/HEAD/src/main/resources/Documentation/syntax.md?pli=1)
to ensure that any changes affecting pkg/analyzer/api.txt will be
reviewed by the developer experience team (or one of the last resort
global reviewers), rather than the Dart Model team.
Since changes to the analyzer that affect its public API will
inevitably affect both .dart files in pkg/analyzer (or
pkg/_fe_analyzer_shared) as well as pkg/analyzer/api.txt, this
ownership change ensures that any change affecting the analyzer public
API will be reviewed both by a member of the Dart Model team and by a
member of the developer experience team.
Change-Id: I07527efe1fa7aa3f1d75488c1800410c2aaaa6ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410660
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The attached bug shows an issue users have been encountering where a constructor seems to be undefined. It turns out this is because DDC is trying to read the constructor from the wrong library.
This is happening because both 'package:dio' and a sister package 'package:dio_web_adapter' both contain a library with the same path: 'src/adapter.dart'. 'BrowserHttpClientAdapter' the class they are trying to reference is defined in 'package:dio/src/adapter.dart'. However, due to a naming collision, their import is referencing 'package:dio_web_adapter/src/adapter.dart'.
This naming collision happens because of the logic in '_jsLibraryAlias'. By truncating the start of the import URI (i.e. 'dio/' and 'dio_web_adapter/') the two libraries map to the same alias. This alias is then used to as the key in the AMD module export object and since both libraries are in the same module, only the second one gets exported.
This code may have been written with the assumption that libraries from different packages would always be in different modules (in which case the shortened paths shouldn't collide) but this is not the case. The fix is to use the full import URI including the package name.
In writing the attached modular test I discovered another issue that only affects es6 imports. The ScopedId resolver was not considering NameSpecifier as a declaration point for variables. This lead to a similar name collision since the import alias's name was also being derived from a truncated import URI. In the test, both 'f1/foo.dart' and 'f2/foo.dart' were being imported 'as foo'. Now one is 'as foo' and the other is 'as foo$'.
The first issue affects both AMD and es6 while the second issue only affects es6. The modular tests run with es6 so the new test fails if either of these fixes is not in place.
The new DDC module system is not affected by either issue since it doesn't use NameSpecifiers and it uses the full import URI as a string to register libraries rather than a shortened alias.
Tested on TGP and with a local Flutter application.
Bug: https://github.com/dart-lang/sdk/issues/56498
Change-Id: I5bdb945cfbe615874b40e2fc4ebba31b661cf3b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410260
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
The Dart language semantics requires global / static fields to be lazily
initialized on first access.
Though if the initializer expression does not have any side-effects, an
implementation may take the liberty to initialize a global earlier. The
downside of that is that if the initializer (despite being side-effect
free) is costly, the startup cost may suffer.
We introduce a `@pragma('wasm:initialize-at-startup')` that allows us to
explicitly opt-into running a global field's initializer at starutp.
This will mean we don't have to pay the lazy-initialization cost at
access time anymore. So when we before did this:
```
block X
global.get GX
br_on_non_null
call initializer
end
```
we now do this:
```
global.get GX
```
we also get rid of the initializer function.
We start to make use of this pragma in 3 places:
* hash map code that checks for deleted marker
* double to string cache
* string interning cache
Change-Id: I172ecda33fad8fab1a02b48b16784f8a9c89d205
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410340
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The code that was previously owned by the analyzer team
(OWNERS_ANALYZER) is now owned by either the Dart Model team
(OWNERS_MODEL) or the developer experience team
(OWNERS_DEVELOPER_EXPERIENCE). Accordingly, we don't need
OWNERS_ANALYZER anymore.
Change-Id: I9a4d2e2462a15ba9ec8f3046cdca77ba9d3af13c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410564
Reviewed-by: Kevin Moore <kevmoo@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Many json files may have small integer numbers in them. Both dart2js and
VM don't need to actually box those small numbers as they have small
tagged integer support. Though dart2wasm boxes all integers when they
flow into top types.
=> We can use a cache of boxed integer numbers <= 255.
It does seem to overall improve json decoding benchmarks and
will reduce memory usage / pressure on the GC.
Change-Id: I026831e0f0841ae84a66652c0cc0e4689a4ab75e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410000
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Currently when a Dart `catch` block can catch both Dart and JS
exceptions, we compile the `catch` body once in a Wasm `catch` block,
once in a Wasm `catch_all` block.
This is because a Dart exception needs to be caught in a `catch` with
the right tag, to be able to get the exception and stack trace values,
and JS exceptions need to be caught in `catch_all` and they come without
error values and stack traces.
With this CL we generate one Wasm block per Dart `catch` block. Wasm
`catch` and `catch_all` blocks only do type tests and jump to the right
Wasm `block` when a type test passes.
This allows using the same block for multiple Wasm `catch` and
`catch_all` blocks.
When jumping to the block for a Dart `catch` we pass the error value and
stack trace to the block. As before, when the caught exception is a JS
exception, we pass an empty `JavaScriptError` as the error value and the
call stack of the Dart `catch` as the stack trace.
We also replace Wasm `rethrow` instruction with `throw` when compiling
Dart `rethrow` statements. This change is necessary as the blocks for
Dart `catch` blocks are no longer enclosed by a Wasm `try`, and it also
makes it easier to switch to the new exception handling proposal, which
doesn't have a `rethrow` instruction.
This changes Wasm exceptions reported to the console in uncaught
exceptions, but when we switch to the new exception handling
instructions we will recover the stack traces, as `throw_ref` doesn't
update the stack trace of the error value.
Change-Id: I732c0192af158611d5f0a584182a48b0e13ff83a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410321
Commit-Queue: Ömer Ağacan <omersa@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The `double.parse()` function first runs a regular expression over the
string to validate it's a valid string representation of a double.
Though the json parser does this kind of validation itself, so we can
avoid this regexp match and call the JavaScript `parseFloat()` function
directly.
Change-Id: Ib2c2c4789d63b75b9a24eac5b5ee2b3483c65327
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410320
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
=> We use wasm arrays to represent the cache used for
stringifcation of doubles.
=> We store unboxed doubles as cache keys and thereby make the cache
have a cache-hit if the double value is the same (before it was a
cache-hit if the double box was identical).
Change-Id: Id944f45188f29659b7704a7db5b321aafbe8c9ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410001
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Moving forward, the Dart Model team will own the following packages:
- _fe_analyzer_shared
- analyzer
- analyzer_cli
- analyzer_utilities
- front_end
- frontend_server
- kernel
- testing
In follow-up CLs I will create an OWNERS file for the developer
experience team, and then remove the OWNERS files for the old analyzer
and CFE teams.
Change-Id: Id7cf38ea41a6d3f9ca44f43fae1f310426a31e29
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409841
Reviewed-by: Kevin Moore <kevmoo@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Moving forward, the Developer Experience team will own the following
packages:
- analysis_server
- analysis_server_client
- analysis_server_plugin
- analyzer_plugin
- language_server_protocol
- linter
- meta
- server_plugin
- telemetry
In follow-up CLs I will create an OWNERS file for the Dart Model team,
and then remove the OWNERS files for the old analyzer and CFE teams.
Change-Id: Icc52ca75e92baf9d05de211e9e3f6c36fb19b71e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409860
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Kevin Moore <kevmoo@google.com>
The summary is checked into source control in
`pkg/analyzer/api.txt`. It contains an entry for each type
declaration, extension, function, method, getter, or setter that is
part of the analyzer API, as well as the types of all functions,
methods, getters, and setters, and all type declarations referenced by
those types.
The tool `pkg/analyzer/tool/api/generate.dart` regenerates
`api.txt`. The test `pkg/analyzer/tool/api/generate_test.dart` checks
that generation of `api.txt` is up to date.
The intention is that `api.txt` will serve as an audit trail to (a)
detect unintentional changes to the analyzer public API, and (b) help
ensure that the developer experience team reviews intentional changes
to the analyzer public API. Once an appropriate OWNERS file has been
set up for the developer experience team, I intend to add an ownership
rule to require developer experience team review for this file.
The structure of the file is hierarchical, with top level nodes
representing libraries URIs, children of those nodes representing top
level elements declared or exported by those libraries, and
grandchildren of those nodes representing members of type
declarations.
If a type is considered part of the API, but it isn't exported by an
analyzer public API library (e.g., the type `Namespace`, which is
declared in `package:analyzer/src/dart/resolver/scope.dart`, but part
of the analyzer API due being exposed by
`LibraryElement.exportNamespace`, `LibraryElement.publicNamespace`,
`LibraryImportElement.namespace`, and so on), then it is listed under
the URI of the library that declares it.
If a type is not considered part of the API, but it is declared inside
`package:analyzer` and it is referenced by something in the API (e.g.,
`Workspace`, which is a private analyzer class but is the type of
`ContextRoot.workspace`), then instead of listing the members of the
type declaration, `api.txt` simply says `(non-public)` after the entry
for the type.
If a type is not considered part of the API, and it is not declared
inside `package:analyzer`, but it is referenced by something in the
API (e.g., `WatchEvent`, which comes from `package:watcher` but is
used in the type of `ResourceWatcher.changes`), then instead of
listing the members of the type declaration, `api.txt` simply says
`(referenced)` after the entry for the type.
If there are two distinct declarations in `api.txt` with the same name
(e.g., the two declarations of `AnalysisContext` from
`package:analyzer/dart/analysis/analysis_context.dart` and
`package:analyzer/src/generated/engine.dart`), then they are
disambiguated by appending an `@` symbol followed by an integer.
Change-Id: Ia2c856e79feebabb74aeb26f57c0a1eea7c63f68
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409862
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Some time ago, the analyzer team 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.
Accordingly, it makes sense to suppress `TODO` comments from being
surfaced to the IDE "problems" view (since there can be a lot of them,
and they're not immediately actionable). This makes VSCode's
"problems" view much more usable in "tree" mode.
(See also https://dart-review.googlesource.com/c/sdk/+/295662, which
made the corresponding change to the `analysis_server` and `analyzer`
packages, https://dart-review.googlesource.com/c/sdk/+/325121, which
made the corresponding change to the `linter` package, and
https://dart-review.googlesource.com/c/sdk/+/358980, which made the
corresponding change to the `analyzer_plugin` package).
Change-Id: I916c8c7f435d06c1b0bd871ef3829d58c9242218
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410060
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Ryan Macnak