In rare circumstances involving syntax errors, the `parsePattern`
method inserts a synthetic token but does not consume any
tokens. Usually when this happens it's not a problem, because whatever
method is calling `parsePattern` consumes some tokens, so the parser
always makes progress. However, when parsing list patterns, after
calling `parsePattern`, the parser would look for a `,`, and if it
didn't find one, it would supply a synthetic `,` and call
`parsePattern` again, resulting in an infinite loop. A similar
situation happened with map patterns, though the situation was more
complex because in between the calls to `parsePattern`, the parser
would also create synthetic key expressions and `:`s.
To fix the problem, when parsing a list or map pattern, after the call
to `parsePattern`, the parser checks whether any tokens were
consumed. If no tokens were consumed, it ignores the next token from
the input stream in order to make progress.
I also investigated whether there were similar issues with
parenthesized/record patterns and switch expressions, since those
constructs also consist of a sequence of patterns separated by tokens
and other things that could in principle be supplied
synthetically. Fortunately, parser recovery doesn't get into an
infinite loop in those cases, so I didn't make any further
changes. But I did include test cases to make sure.
Fixes#52352.
Bug: https://github.com/dart-lang/sdk/issues/52352
Change-Id: Idc8140236f6054deb1fd3c862036fe47dd84f30b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302803
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
to the list.
This should fix https://github.com/dart-lang/sdk/issues/52364
Change-Id: Id6ab25ee48da4027adee89e91868bde5b07b4ac2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303002
Reviewed-by: Kevin Moore <kevmoo@google.com>
Commit-Queue: Jackson Gardner <jacksongardner@google.com>
Reviewed-by: Mouad Debbar <mdebbar@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
I am concerned about the tests marked with TODOs because I don't know
why the behavior changed. It's possible that the contributor is still
producing the suggestions but that they're being filtered out later.
I checked using the debugger, and they're not in the list of results
that we're getting back, so it seems likely that they represent real
bugs. If we just weren't detecting the bug before then we're good.
But I can't think how rewriting the tests could effect what the user
is seeing, so I think it's safe to make these changes.
Change-Id: If72042b7ec24b97994a12449c811213b8f708f72
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303000
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Update the format of the shape keys to better match the format used in
the dart:_rti library. This change applies to the current runtime type
system as well.
Change-Id: I87d2af2aaf2b9dbe012fae60a64718d264a3a18c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295721
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Also includes https://dart-review.googlesource.com/c/sdk/+/302680
that was reverted because of adding extre diagnostics for Flutter.
Change-Id: I4cf7cedfcb67a5af226e609d279551fea0b0dee1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302844
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This cache didn't use the head size as intended.
Closes#52345
Change-Id: I99a3e4fe1664186103d889e0a324b6992947216a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302922
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
The map `breakFinalizers` need to have insertion-ordered iteration
otherwise creation of Wasm blocks when compiling try-finally blocks
won't work. Update the map type from `Map` to `LinkedHashMap` to make
this clear.
This doesn't change the concrete map type used as non-constant map
literals are already compiled to `LinkedHashMap`s, just makes the type
more precise for clarity.
Change-Id: Idebffe2f0f75232f5ef41729538ad3d2508c74c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294760
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Change-Id: I907345b2bfeedebc87cb79931b5eece38ef0f576
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290611
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Change-Id: I061fa10156ec665b9296211588313f45c850cec1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300041
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
I'm becoming concerned that the filtering is making it difficult to
verify the results. We can look at the textual output to see whether all
of the printed suggestions are valid, but we can't know what other
suggestions are being produced and whether they're valid, nor can we be
sure that suggestions that shouldn't be produced are not being produced.
Change-Id: Id131b9b53740b625ba74a9b5bdb690da21fcdbfc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302901
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
These tests are flaky or timing out on the stable DDC configurations.
This change adds skips for the canary configurations to avoid crashing
the infra when too many tests that timeout are attempted to be
deflaked.
When the canary mode stabilizes we should manually mark all these
tests as flaky if they are still not passing on the stable DDC
configurations.
Issue: https://github.com/dart-lang/sdk/issues/50666
Change-Id: I9b1cb8fe466624480767fea05610331bf0f4ed87
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302843
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
TEST=docs only
Change-Id: Ib2921c485defc232264d0c4f86b783ce33c6eadd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302848
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
These were introduced in
https://dart-review.googlesource.com/c/sdk/+/287601 to allow the
common front end to report back to the shared analysis logic when it
desugars a guard expression, so that flow analysis will properly
account for any promotions made by the guard expression; previously,
if a guard expression got desugared by the front end, promotions
performed by the guard would be lost.
In https://dart-review.googlesource.com/c/sdk/+/298840, we've switched
to a more general technique for ensuring that promotions performed by
the guard will not be lost: now the `dispatchExpression` method uses
`FlowAnalysis.forwardExpression` to compensate for the differences in
the conventions of the CFE and the shared logic, so that the shared
logic no longer needs access to the desugared expression. So the
`head` parameter (and the associated return value) are no longer
needed.
In a future change, I plan to adjust the CFE conventions to match
those of the shared logic, so that `FlowAnalysis.forwardExpression`
won't be needed at all (see
https://github.com/dart-lang/sdk/issues/52189).
Change-Id: I686d1d5b7e4c9db353f583c38792274451f9f8bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302366
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Additionally, the CL adds a runtime check for handling all of the type
builder kinds in the function for type variable usage detection.
Closes https://github.com/dart-lang/sdk/issues/52339
Change-Id: I84c7b53f4f0cfc39a18356d5b35fbdc843bb8a5b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302740
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Maintains functionality of existing '--write-X-uri' and '--read-X-uri' but also introduces new '--stage' flag that indicates which phases of the compiler should run.
Also expands the behavior of the `--output` flag to support taking a file prefix and/or an output directory that files are written out to.
Valid invocations can look like:
`dart2js main.dart --stage=cfe`
`dart2js main.dart --stage=cfe --output=prefix1-`
`dart2js main.dart --stage=cfe --output=/some/path/`
Change-Id: If90fb16f549d35f16f77d660c51f8d28673e454f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300520
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This change locks down the Dart2JS command line API to the few invocation flows we use today:
1) Full compile from Dart sources.
2) Full compile from pre-compiled dill sources (modular CFE).
3) Phased compilation from Dart sources.
4) Phased compilation from pre-compiled dill sources (modular CFE).
It also ensures that each phase is run individually (accounting for sharding in codegen/emitter).
There are already places in the code where we assume that Dart2JS will be invoked in one of these ways so this should codify those assumptions.
Change-Id: I4e2a85d123e0c2ad924e2dd1b230548a995af0da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299760
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
In d592882f, the old uint8_t `type_state_` and `nullability_` fields
in subclasses of `AbstractType` were merged into a single uint32_t
`flags_` field in `AbstractType` itself.
Accessing the nullability information was abstracted into two methods:
* `LoadAbstractTypeNullability`
* `CompareAbstractTypeNullabilityWith`
However, the code from these methods were based off the old
`CompareTypeNullabilityWith` and `CompareFunctionTypeNullabilityWith`
methods that used unsigned byte loads to access the old field, and kept
that even though the new field is now a 32-bit unsigned integer. This
works for now, because the nullability portion of the `flags_` field
happens to be the lowest two bits, but might lead to confusion if
someone attempts to write similar code for accessing the other parts of
the `flags_` field without realizing it's larger than 8 bits.
Thus, this CL does the following:
* Unifies the implementation of both methods across architectures.
* Adds a native slot definition for the `AbstractType::flags_` field
and uses `LoadFromSlot` to load the `flags_` field instead. Other
places where the `AbstractType::flags_` field is accessed from the
assembler are also changed to use `LoadFromSlot` appropriately.
TEST=The existing suite of tests are sufficient.
Change-Id: Ie578e4dddce810223ca9e767403136cb40d340f3
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-release-x64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-release-simarm64-try,vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-debug-simriscv64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-linux-debug-x64c-try,vm-kernel-precomp-linux-release-simarm-try,vm-linux-release-x64-try,vm-linux-release-simarm64-try,vm-linux-release-simarm-try,vm-linux-release-ia32-try,vm-linux-debug-simriscv64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302187
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
When a post-feature library implements a pre-feature library declaration that has a final core library class as a super declaration, it should not produce a base/final/sealed transitivity error. Subclasses of a base core library class will still emit this error.
Otherwise, if base/sealed/final is omitted in a non-implement case, we want to report these errors.
Bug: https://github.com/dart-lang/sdk/issues/52315
Change-Id: Icdd4f63f69b061be5eabc7fd30b703d0358018a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302365
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This has a few changes. One of them affects code even before Dart 3,
though it only touches parameters with metadata on them, which tend to
be rare (outside of uses of the freezed package). It may affect code in
the SDK repo.
The other two are specific records and patterns so won't affect much
existing code. But the formatted behavior before these bug fixes looks
pretty bad, so if it's not too much risky, I think it's worth cherry
picking into the next stable point release.
Change-Id: I1e60d251b474b099c5059ffa63ddb260f0ed1318
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302500
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Michael Thomsen <mit@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
With one exception (noted below), the shared analysis logic uses the
convention that the expressions passed to flow analysis are the
original (pre-lowered) expressions, whereas the expressions passed to
flow analysis by the CFE are the lowered expressions. This difference
caused two problems:
- If a boolean expression appeared on the right hand side of a pattern
assignment or pattern variable declaration, and it was lowered, the
flow analysis implications of that boolean expression would be lost.
- If a boolean expression appeared in a `when` clause, and it was
lowered, the flow analysis implications of that boolean expression
would be lost. Exception: for switch statements, the shared analysis
logic has been passing lowered expressions to flow analysis, so this
problem didn't occur for `when` clauses in switch statements.
Notably, when compiling for the VM, the CFE lowers expressions like
`x != null` to something more like `!(x == null)`.
Fortunately, the first of these two situations shouldn't cause
problems very often, since typically if the right hand side of an
assignment or variable declaration is a boolean expression, there is
no need for the left hand side to involve patterns.
As for the second of these two situations, it's also not too likely to
cause problems, since typically null checks occur inside patterns
rather than in `when` clauses.
As a short term fix, we remove the exception noted above, and we
account for the difference in conventions by adding a call to
`FlowAnalysis.forwardExpression` to the CFE's implementation of
`dispatchExpression`, so that when transitioning between CFE logic and
shared logic, flow analysis will be informed how to match up the
lowered expressions to their pre-lowered counterparts.
Longer term, I would like to switch everything to the convention of
using the original (pre-lowered) expressions; this will bring the
analyzer and CFE into better alignment with each other and should
eliminate a class of subtle bugs. This long term goal is tracked in
https://github.com/dart-lang/sdk/issues/52189.
Fixes#52183.
Fixes#52241.
Bug: https://github.com/dart-lang/sdk/issues/52183, https://github.com/dart-lang/sdk/issues/52241.
Change-Id: I2449ce34c54325603bc2730d1660a7cfc7d79aec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298840
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The [ReloadOperationScope] has [StackResource]s as fields and is itself
a [StackResource] which is problemantic if unwinding happens manually.
So we'll make it a macro that expands to the 3 fields instead.
TEST=ci
Change-Id: I3fb7bec7ca87193c83ec34908f9a43c5db005900
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302201
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
TEST=Checked that traces written using the Perfetto file recorder, and
traces retrieved through `getPerfettoVMTimeline` still looked correct.
Change-Id: I7e327ef525c99187fa8aea868d1cc48721af9f98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302420
Reviewed-by: Ben Konyi <bkonyi@google.com>
