This changes the body builder to create an InvalidExpression for new expressions in constant contexts, thus avoiding the need to handle this case in the inference, avoiding the need for passing constantContext to the inferrer.
Change-Id: Ia8d5b3a6462e7a1570a092d5808bf262345b424b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451021
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Some CFE lowerings (e.g. pattern lowerings) result in nested scopes containing VariableDeclarations with the same 'name'. The current DDC transform translates these to the exact same name in JS leading to incorrect semantics.
The `TemporaryId` mechanism automatically renames any variables with the same name that would shadow each other. So we re-use that here to ensure the variables all have a unique name if the CFE hasn't already given them one. If the name is already okay (i.e. not shadowing something else), the name in JS will appear unchanged.
Side note: In a future change perhaps we should rename `TemporaryId`. The general mechanism it implements is more useful than its original intended use.
Fixes: #59613
Change-Id: I708c72528d5df19af48dde01163d375a5588baae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398504
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Collects files from `package:async_helper` and `tests/language`
that are generally useful, so that all test-related helpers are
in `package:expect`.
Moves the two libraries from `package:async_helper` into `package:expect`,
and the `tests/language/static_type_helper.dart` file too.
Deprecates `async_minitest.dart`, to follow `minitest.dart`,
expecting the Flutter use of it to have been fixed to not break
on deprecation (I believe Flutter no longer breaks builds on deprecations at all).
Patch 1 is the actual change.
Patch 2+4+8 is changing all existing references to the files.
Patch 6 ignores deprecation in files still using `async_minitest.dart`.
3+5+7+9 are updating this text to make the numbers match.
Then it's just test-expectations and small tweaks from there.
Change-Id: I1b665135b5fef9b9a0c3b340ffe9daf874d0174c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373120
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
When determining whether a switch statement is exhaustive, it's
important for the exhaustiveness algorithm to ignore cases containing
`when` clauses, since a `when` clause creates the possiblity that the
case won't match.
Previously, the way this was done in the analyzer was for the
`SpaceCreator.createRootSpace` method to create an unknown space for
cases containing `when` clauses. This approach produced the correct
behavior when determining whether the switch statement as a whole was
exhaustive, but since it discarded information about the pattern being
matched, it limited the ability to determine whether an individual
case was reachable, leading to
https://github.com/dart-lang/sdk/issues/56710.
To fix this, `SpaceCreator.createRootSpace` is changed so that it
always produces a space that describes the case pattern, regardless of
whether a `when` clause is present, and instead,
`computeExhaustiveness` is responsible for ensuring that the case is
properly excluded from the determination of whether the switch is
exhaustive. This allows `computeExhaustiveness` to properly computate
whether each individual case is reachable, even for cases that have
`when` clauses.
This change in approach produced some minor differences in the test
cases in `pkg/_fe_analyzer_shared/test/exhaustiveness/data`, but these
differences are not user-observable.
Fixes https://github.com/dart-lang/sdk/issues/56710.
Change-Id: I36629a77c4c1832fb1b8abb6ea7b109e0ca14373
Bug: https://github.com/dart-lang/sdk/issues/56710
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384326
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This test was out of place (since it doesn't contain any variable
patterns). Also, it was an exact copy of a test in
record_literal_switch_test.dart. There's no point in having the same
test in both locations, so remove it from the location where it
doesn't belong.
Change-Id: I1d3a786caba2b2591255b68c035eff40b2f08237
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384565
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
This warning is similar to the existing `UNREACHABLE_SWITCH_CASE`
warning, except that it warns if the `default` clause of a switch
statement is unreachable due to all the `case` clasuses fully
exhausting the switched type.
To make the implementation easier, I changed the API for the
`reportExhaustiveness` method in `_fe_analyzer_shared` (which is the
primary entry point to the shared exhaustiveness checker). Previously,
this method returned a list of `ExhaustivenessError`, where each list
element was either an `UnreachableCaseError` (indicating that a
certain case was unreachable) or a `NonExhaustiveError` (indicating
that the entire switch statement was not exhaustive). If the caller
passed in `false` for `computeUnreachable`, `UnreachableCaseError`s
would not be returned, so the returned list would either be empty or
contain a single `NonExhaustiveError`.
The new API renames the types for clarity:
- `NonExhaustiveError` becomes `NonExhaustiveness`, to highlight the
fact that it's not necessarily an error for the switch's cases to be
non-exhaustive; it's only an error if the scrutinee's static type is
an "always exhaustive" type and there is no `default` clause.
- `UnreachableCaseError` becomes `CaseUnreachability`, to highlight
the fact that it's not an error for a case to be unreachable; it's a
warning.
Also, the new API adds instances of `CaseUnreachability` to an
optional user-provided list instead of returning a newly created list;
this allows callers to communicate that they don't need to see
`CaseUnreachability` information by passing `null`. This frees up the
return type to simply be an instance of `NonExhaustiveness` (if the
cases are not exhaustive) or `null` (if they are exhaustive). This
makes it easier for the analyzer to decide whether to issue the new
warning, because it doesn't have to dig around the list looking for an
instance of `NonExhaustiveness`.
The new warning has an associated quick fix (remove the unreachable
`default` clause). This quick fix uses the same `RemoveDeadCode` logic
in the analysis server that the existing `UNREACHABLE_SWITCH_CASE`
warning uses.
Fixes https://github.com/dart-lang/sdk/issues/54575.
Bug: https://github.com/dart-lang/sdk/issues/54575
Change-Id: I18b6b7c5249d77d28ead7488b4aae4ea65c4b664
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378960
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Previously, `HintCode.UNREACHABLE_SWITCH_CASE` was marked as
deprecated, and `WarningCode.UNREACHABLE_SWITCH_CASE` was an alias to
it. This created a slightly confusing situation, because it meant that
all code referring to the diagnostic had to refer to it as
`WarningCode.UNREACHABLE_SWITCH_CASE` (to avoid a deprecation lint),
but the diagnostic still _behaved_ like it was a hint, and therefore
test runner expectations still had to treat it as a hint.
It turns out that it's not really necessary to go through the
deprecation dance when changing the kind of a diagnostic, since (a)
members of `HintCode` and `WarningCode` aren't exposed through the
analyzer public API, and (b) ignore comments don't have to specify
whether something is a hint or a warning.
So the easiest way to clear up the confusion is to just remove
`HintCode.UNREACHABLE_SWITCH_CASE` entirely, and move its implemention
into `WarningCode.UNREACHABLE_SWITCH_CASE` (so that the latter is no
longer an alias).
Change-Id: I9ff7901ad38a2c168c5e54cbe0c1c52bf7c50186
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381103
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
No client of the VM uses this flag, only tests, and this flag was always
set to false in AOT mode. Thus, remove uses of this flag and instead
always lazily create dispatchers as needed when resolving method names
in JIT mode.
Remove the implicit value of `allow_add` for some Resolver
static methods. For callers that previously depended on the implicit
`true` value (which includes the AOT precompilier), pass `true` for
uses in the compiler and pass `!FLAG_precompiled_mode` for uses in the
runtime. Assert that `allow_add` is false when these methods are invoked
from the precompiled runtime.
Remove Resolver static methods that are no longer used.
TEST=ci
Change-Id: Ib6a7354f7a859e86743c381513a4129c14895753
Cq-Include-Trybots: luci.dart.try:vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-mac-release-arm64-try,vm-mac-debug-arm64-try,vm-mac-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366668
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
When analyzing the type test implied by a pattern, flow analysis uses
three variables to control promotion behavior:
- `matchFailsIfWrongType`, which indicates whether flow analysis needs
to account for the possible control flow path resulting from the
type test failing. (This is `false` for cast patterns, because in
the case where a cast pattern fails, an exception is thrown).
- `matchMayFailEvenIfCorrectType`, which indicates whether flow
analysis needs to account for the possible control flow path
resulting from the type test succeeding, but some other check
causing the match to fail. (This is `true` for most list patterns,
because the list pattern will fail to match if the list has the
wrong length).
(Note that `matchMayFailEvenIfCorrectType` doesn't account for the
fact that a pattern match might fail due to failure in a subpattern
match; this is automatically handled by the fact that flow analysis
walks through the complete pattern in the order in which it
executes.)
- `coversMatchedType`, which indicates whether the type test is
guaranteed to succeed due to a subtype relationship between the
matched value type and the type being tested (e.g. a `num x` pattern
is guaranteed to succeed if the matched value type is `int`).
In the case where `matchFailsIfWrongType` is `true`,
`matchMayFailEvenIfCorrectType` is `true`, and `coversMatchedType` is
`false`, flow analysis must account for the fact that there are two
ways that the pattern match might fail: the type test might fail, or
the type test might succeed but then the pattern match might fail for
some other reason.
Before this change, this was done incorrectly, and flow analysis only
accounted for the possibility of the type test failing.
Fixes#55543.
Bug: https://github.com/dart-lang/sdk/issues/55543
Change-Id: I86603ec5f940402313f32177212b7960878db97f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364942
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
According to the patterns spec, the pattern context type schema for a
cast pattern should be `_`. What was actually implemented was
`Object?`.
This is unlikely to make a difference in practice, since (a) cast
patterns are unlikely to be used in circumstances where the pattern
context type schema makes a difference, and (b) it takes some effort
to come up with expressions whose type inference behavior is differenc
between a schema of `Object?` and a schema of `_`.
Change-Id: I695752c8c163621a34faaa8d62b2b076c8152eb0
Bug: https://github.com/dart-lang/sdk/issues/54640
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346383
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This was a mistake in the shared logic implementation during the
implementation of patterns; instead of verifying that the for-in
expression is a subtype of `Iterable<dynamic>` (or `Stream<dynamic>`
in the case of an `await for`), it merely verifies that it's a subtype
of `Iterable<dynamic>?` (or `Stream<dynamic>?`). At some point, the
requisite error checking was added to the front end, but not to the
analyzer.
In an ideal world, the solution would be to remove the error check
from the front end and add it to the shared logic; however, doing so
is not easy, because what needs to be done is an assignability check
(as well as insertion of implicit downcasts and other coercions). The
shared logic doesn't currently have a good hook for doing that. So,
for now, this CL simply adds the requisite check to the analyzer. I
plan to try to find a clean way to share some of this logic in a
follow-up CL.
Fixes https://github.com/dart-lang/sdk/issues/54671.
Change-Id: Ib2fa8bce758dfe12401d85850666674ca1317e3f
Bug: https://github.com/dart-lang/sdk/issues/54671
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347360
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I discovered recently that when we implemented the "patterns" feature,
we got the pattern type schema for cast patterns wrong--it was
specified to be `_`, but what was actually implemented was
`Object?`. I've filed a breaking change request to address that:
https://github.com/dart-lang/sdk/issues/54640.
In the course of prototyping a fix, I was surprised to find that no
language tests or co19 tests were affected; it appears that we don't
have adequate test coverage of pattern type schemas. This CL addresses
the lack of test coverage by adding a language test.
The test case for cast patterns is currently commented out, pending
resolution of the breaking change request; after the breaking change
lands, I will update the test accordingly.
Change-Id: I0d27fd8ac65f16d21a8597586e6f76fd9a5ba86e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346621
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This moves the hoisted declaration of pattern variables till after the
variable cache declarations, such that any use of variables in the
matched expression, shadowed by the pattern variables, occurs before the
declaration of the shadowing variables.
Closes#54559
Change-Id: Id8ca8e7a499b9d71a50cd9987808d159f26bbd24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345942
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This adds a new rule to exhaustiveness handling of the as-pattern. The
new rule checks whether the cast type is fully contained in the space
of the subpattern. If so, we can assume that the whole context type has
been matched because there is no value that passes the as-test that
would be rejected by the subpattern.
Closes#51986Closes#54125
Change-Id: I0dc7fb072395aa3bc9f0b143afb320966f9d64c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338760
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This change reverts https://dart-review.googlesource.com/c/sdk/+/341020.
The reverted CL removed tests from `tests/language` that contained
legacy code (code with a language version less than 2.12). The primary
purpose of the removal was to unblock removal of legacy support from
the analyzer. It seemed safe to do because legacy code isn't supported
anymore, and no legacy code exists in google3 anymore. However,
several of those tests exercised important "weak mode" runtime
semantics of the web and VM platforms. See discussion at
https://dart-review.googlesource.com/c/sdk/+/341020/comments/bce31aa1_f5392ce1
for details.
This CL restores the deleted tests. Fortunately, since all the
restored tests are annotated with `Requirements=nnbd-weak`, they will
be skipped on the analyzer (thanks to
https://dart-review.googlesource.com/c/sdk/+/342080), so restoring
them no longer gets in the way of removing legacy support from the
analyzer.
Change-Id: Ib828be76f5c7eaeecaad0b7e7f7b0e3ff2f4bdb2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342090
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Now that we no longer need to support legacy (pre-2.12) code in
google3, we don't need to test our support for it either.
Most of our language tests for legacy code were in `tests/language_2`
and have already been deleted; this CL addresses the few tests that
remain in `tests/language`, mostly dealing with "mixed mode" scenarios
(i.e., interactions between legacy code and null safe code).
Change-Id: I023d84547fdf17fb0599c82fbeb4fd07252ccf0f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341020
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, the flow control logic for patterns didn't use the
`FlowModel.split` or `FlowModel.unsplit` methods at all. This meant
that if a control flow join point occurred in pattern logic, flow
analysis would consider the split point to be whatever split point was
established by the enclosing expression or statement. In the case of
an if-case statement, it would consider the split point to be at the
beginning of the scrutinee expression.
Split points are used by flow analysis for the sole purpose of
ensuring that joins propagate type promotions the same way in dead
code as they do in live code (so that users introducing temporary
`throw` expressions or `return` statements into their code do not have
to deal with nuisance compile errors in the (now dead) code that
follows. The consequence of flow analysis considering the split point
to be at the beginning of the scrutinee expression is that if the
scrutinee expression is proven to always throw, then joins that arise
from the pattern or guard may not behave consistently with how they
would have behaved otherwise. For example:
int getInt(Object o) => ...;
void consumeInt(int i) { ... }
test(int? i) {
if (
// (1)
getInt('foo')
case
// (2)
int()
// (3)
when i == null) {
} else {
// (4)
consumeInt(i);
}
}
In the above code, there is a join point at (4), joining control flows
from (a) the situation where the pattern `int()` failed to match, and
(b) the situation where `i == null` evaluated to `false` (and hence
`i` is promoted to non-nullable `int`). Since the return type of
`getInt` is `int`, it's impossible for the pattern `int()` to fail, so
at the join point, control flow path (a) is considered
unreacable. Therefore the promotion from control flow path (b) is
kept, and so the call to `consumeInt` is valid.
In order to decide whether to preserve promotions from one of the
control flow paths leading up to a join, flow analysis only considers
reachability relative to the corresponding split point. Prior to this
change, the split point in question occurred at (1), so if the
expression `getInt('foo')` had been replaced with `getInt(throw
UnimplementedError())`, flow analysis would have considered both
control flow paths (a) and (b) to be unreachable relative to the split
point, so it would not have preserved the promotion from (b), and
there would have been a compile time error in the (now dead) call to
`consumeInt`.
This change moves the split point from (1) to (2), so that changing
`getInt('foo')` to `getInt(throw UnimplementedError())` no longer
causes any change in type promotion behavior.
The implementation of this change is to add calls to `FlowModel.split`
and `FlowModel.unsplit` around all top-level patterns. At first glance
this might appear to affect the behavior of all patterns, but actually
the only user-visible effect is on patterns in if-case statements,
because:
- In switch statements and switch expressions, there is already a
split point before each case.
- In irrefutable patterns, there is no user-visible effect, because
irrefutable patterns cannot fail to match, and therefore don't do
any control flow joins.
This change allows the split points for patterns to be determined by a
simple syntactic rule, which will facilitate some refactoring of split
points that I am currently working on.
Change-Id: I55573ba5c28b2f2e6bba8731f9e3b02613b6beb2
Bug: https://github.com/dart-lang/sdk/issues/53167
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319381
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Prior to this change, the first phase of flow analysis, in which flow
analysis "looks ahead" to see which variables are potentially assigned
in each code block, was failing to properly account for variables that
are assigned by pattern assignment expressions. This "look ahead"
information is used by flow analysis to determine the effects of
nonlinear control flow (e.g. to figure out which variables to demote
at the top of a loop, or to figure out which variables are potentially
assigned inside a closure). As a result, it was possible to construct
correct code that would be rejected by the analyzer and compiler, as
well as incorrect code that would (unsoundly) be accepted.
The fix is to call `AssignedVariables.write` from the analyzer's
`FlowAnalysisVisitor`, and from the CFE's `BodyBuilder`, to let flow
analysis know about variable assignments that occur inside pattern
assignment expressions.
Fixes#52745.
Change-Id: I0e6f426a5c5c36f214d5a206aaf5a2de0bfdaac1
Bug: https://github.com/dart-lang/sdk/issues/52745
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310502
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@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>
In https://dart-review.googlesource.com/c/sdk/+/299400, the parser was
adjusted so that it no longer accepts `when` and `as` as the names for
variable patterns in cases where there is a possible ambiguity
(e.g. `int when` is not accepted as a pattern because `int` is a
legitimate pattern, therefore `when` could introduce a guard
clause). This change further prohibits `when` and `as` from being the
names of variable patterns or identifier patterns even in the case
where there is no ambiguity. This is in line with the discussion at
https://github.com/dart-lang/sdk/issues/52199#issuecomment-1526297771,
and the spec change at
https://github.com/dart-lang/language/pull/3033.
Three new error codes are introduced, to cover the three circumstances
in which `when` or `as` might be used illegally: in a declared
variable pattern, in an assigned variable pattern, or in an identifier
pattern. I've also added analyzer tests to ensure that the parser
recovers from these errors nicely. Unfortunately, nice error recovery
is only feasible in the non-ambiguous cases.
I've also updated the language test expectations in
`tests/language/patterns/version_2_32_changes_error_test.dart` to
reflect the new error messages, and added a few more examples of uses
of `when` and `as` that are still permitted.
Fixes#52260.
Bug: https://github.com/dart-lang/sdk/issues/52260
Change-Id: I229f627aa639659c30b83c74895759207da279f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301482
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change fixes parsing of case clauses such as:
case foo when !flag:
Constructions like this require some lookahead in order to parse
correctly, because the token `when` is valid both as an identifier and
as a part of the grammar for a case clause. Therefore, at the time
`foo` is encountered, the parser must decide whether it is looking at
a variable pattern (`foo when`, where `when` is the name of the
variable) or an identifier pattern (`foo`, where `when` begins the
case's guard clause). Previous to this fix, the algorithm for
disambiguating these two choices was as follows:
- If the token sequence starting at `foo` looked like a type, and the
token that follows was an identifier, the parser assumed it was
looking at a variable pattern with a type; otherwise it assumed it
was looking at an identifier pattern.
- EXCEPT that if the token that followed the supposed type was `when`
or `as` (both of which are valid identifiers), then it probed
further:
- If the token that followed `when` or `as` was a token that could
legitimately follow a pattern, then it assumed that it was looking
at a variable pattern with a type. (The tokens that could
legitimately follow a pattern are `,`, `:`, `||`, `&&`, `)`, `}`,
`]`, `as`, `when`, `?`, `!`).
- Otherwise it assumed that it was looking at an identifier pattern.
This didn't fully disambiguate, because the third bullet didn't
account for the fact that the tokens `as`, `when`, and `!` could
_either_ legitimately follow a pattern _or_ legitimately begin an
expression (or, in the case of `when`, a type), therefore constructs
like the following were incorrectly parsed:
- `case foo when as:` (where `as` is a local boolean variable)
- `case foo when when:` (where `when` is a local boolean variable)
- `case foo when !flag:` (where `flag` is a local boolean variable)
- `case foo as when:` (where `when` is the name of a type)
The solution is to simplify the disambiguation logic so that if if the
token that follows the supposed type is `when` or `as`, then the
parser assumes that it's looking at an identifier pattern, _not_ a
typed variable pattern.
The consequence of this is that the above four constructions are
parsed correctly; however it is no longer possible for a typed
variable pattern to name a variable `when` or `as`.
For consistency we would like to prohibit _any_ variable pattern from
naming a variable `when` or `as`, however to keep this change as small
as possible (and reduce the risk involved in a possible cherry-pick)
that will be postponed until a later CL.
Fixes#52199.
Bug: https://github.com/dart-lang/sdk/issues/52199
Change-Id: Ibab9b92f01e3e4020d7d64f1ff000a9b964a4564
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299400
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Previously this step happened during `buildOutlineNodes`, but since
`buildOutlineNodes` happens in source order, this means that anonymous
mixins would only get their sealed and final attributes properly
inferred if they appeared *after* their immediate supertypes in source
order. By moving this step to `checkSupertypes`, we ensure that the
computation is correct regardless of source order, because
`checkSupertypes` happens in class hierarchy order.
Fixes#52048.
Bug: https://github.com/dart-lang/sdk/issues/52048
Change-Id: Ib9f1f3dafded88681a26f09e4d21dfd44e70dfd3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297901
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This fixes a minor bug in flow analysis which was preventing it from
recognizing when a switch statement was trivially exhaustive, meaning
one of its reachable cases was guaranteed to always match.
This mostly addresses
https://github.com/dart-lang/language/issues/2980, but flow analysis
still fails to recognize that:
- A list pattern containing a just a single rest pattern always
matches (unless the rest pattern has a subpattern that may fail to
match).
- A null check pattern always matches if its subpattern always matches
and the matched value type is non-nullable.
- The relational pattern `!= null` always matches if its subpattern
always matches and the matched value type is non-nullable.
Fortunately, these drawbacks are small and don't lead to unsoundness.
I'll try to address them in follow up CLs.
Bug: https://github.com/dart-lang/language/issues/2980
Change-Id: Ie9f8564cde66a5a2c41114033ca3ff0e1a0f139a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293860
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Johnni Winther