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>
Also update the test name to follow test naming conventions (tests
with error expectations are named `*_error_test.dart`), and add a
copyright notice and test description.
Bug: https://github.com/dart-lang/sdk/issues/52202
Change-Id: I762dcbf6ebd02190250ccdd9767e6db29cac1d61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300322
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@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>
If a list pattern consists of a single rest pattern, and that rest
pattern is guaranteed to match, then the whole list pattern is
guaranteed to match as well (provided that the matched value type is a
subtype of the list pattern's required type).
Bug: https://github.com/dart-lang/language/issues/2980
Change-Id: I316cc93d4e696f094716be92e1fbc1cd3a43a73c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294622
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@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>
This change ensure that if the user makes an erroneous switch
expression by imitating the syntax of case statements (i.e. using
`case`, `default`, `:` instead of `=>`, or `;` instead of `,`), error
recovery will understand the user's intent, avoiding follow-on errors.
Fixes#51886.
Bug: https://github.com/dart-lang/sdk/issues/51886, https://github.com/dart-lang/sdk/issues/51943
Change-Id: Icd405d4fd4ddfb1aadcf0867e5a51ba898d4cdbc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293200
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This updates the message report for non-exhaustive switch statements
and expressions to include the witness in the problem message and
a reduced witness, which doesn't include properties that fully cover the
static type. The message is also split into two messages; one for
switch statements and one for switch expressions, allowing for a
better wording regarding the default/wildcard pattern case.
Change-Id: I17db657ef12ade5d47fa96bf69b8807e33ed5b8c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293040
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
According to the spec, the expression inside a relational pattern is
`bitwiseOrExpression`. We were only allowing `shiftExpression`.
Bug: https://github.com/dart-lang/sdk/issues/50035
Change-Id: Ie1a5746f1060b84e6e1b856a622e89db698b4684
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292285
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
The implementations are doing the right thing, but the tests are
slightly off:
- The CFE doesn't report unreachable case warnings because they are
non-fatal warnings, so remove those expectations. (But analyzer does,
so continue to check that analyzer reports them.)
- The CFE reports shared case variable errors at a different location.
Change-Id: I7143a7705d3c8879fb91e5fdb8df8599bfb96165
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292520
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Just corner cases that occurred to me while specifying the
implementation. There's probably already front end tests for it too, but
I like the idea of having some language coverage as well.
Change-Id: I158f274483799c3ccb29c32f0e7c5c42d31f327f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291962
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Eventually, these hints should probably be moved over to warnings. But
for now, this makes it possible to write static error language tests
that validate that analyzer produces unreachable case warnings/hints
where expected.
Also updated the patterns/ and switch/ tests now that those errors must
be expected by the test.
Change-Id: If1fb92602c4bde2819b9eec73598033009054947
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291967
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The precedence-based pattern parser can understand a unary pattern
inside another unary pattern (e.g. `_ as int as num`) or a relational
pattern inside a unary pattern (e.g. `> 1?`), but the specification
prohibits these constructions because they're difficult to read and
not very useful.
This change updates the implementation to match the spec, by producing
the appropriate error. The offset and length of the error cover the
inner pattern, so it should be easy to construct an analysis server
quick fix that inserts the necessary parentheses.
Bug: https://github.com/dart-lang/sdk/issues/50035
Change-Id: I33e74d6d1f863e7162851d26fefbacd4fd17277c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292120
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The logic for parsing types has special disambiguation rules for
deciding whether a trailing `?` should be included in the type, based
on what token(s) follow the `?`. In the case where the token that
follows the `?` is `when`, we need to look further ahead to
disambiguate, to distinguish `PATTERN as T? when guard` from something
like `EXPRESSION is T ? when : otherwise`.
(Note: an alternative implementation would be to disambiguate based on
whether we're parsing a pattern or an expression. But in the future I
want to move toward an architecture where expression parsing and
pattern parsing are combined, so that if the parser makes the wrong
decision about whether it's looking at a pattern or an expression,
error recovery will do a better job. So I'm disambiguating based
solely on what follows the `?`.)
Bug: https://github.com/dart-lang/sdk/issues/50035
Change-Id: Idbc780b7b54fecc7fd01cae868c34771564dd804
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292282
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
It is still an error to have duplicate keys.
Change-Id: Id86fa7b50c3620540f4bef0399f5f70316848662
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292125
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The listener API for variable patterns is split into three separate
functions, to handle the three separate behaviors:
- `handleAssignedVariablePattern` for variable names appearing in an
assignment context (these assign to an existing variable upon a
successful match).
- `handleDeclaredVariablePattern` for variable declarations appearing
in a declaration or matching context (these cause a new variable
name to come into scope).
- `handleWildcardPattern` for wildcards in any context (these don't
capture the matched value).
Also, responsibility is shifted to the parser for reporting the
following error conditions:
- VariablePatternKeywordInDeclarationContext (e.g.
`var (var x) = ...;`)
- PatternAssignmentDeclaresVariable (e.g. `[x, var y] = ...;`)
Previously these errors were detected by the implementations, and
weren't fully covering all possible error scenarios.
In the case of VariablePatternKeywordInDeclarationContext, the
listener method `handleDeclaredVariablePattern` is called instead of
`handleAssignedVariablePattern`. This ensures that no tokens are
dropped from the analyzer AST. The CFE uses the `inAssignmentPattern`
argument of `handleDeclaredVariablePattern` to distinguish this error
recovery case from a legitimate declared variable pattern.
Fixes#51868.
Bug: https://github.com/dart-lang/sdk/issues/51868
Change-Id: I28ec679b73d64033166721c6460be35f15e23171
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291583
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: I2cc53a7f41721fd146051502ee2f9e102656ed61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291968
Auto-Submit: Leaf Petersen <leafp@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
I've also added some runtime checks to
`tests/language/patterns/object_pattern_inference_test.dart` to verify
that the object patterns match when they should match and don't match
when they shouldn't; this helps verify that not only has the front end
inferred the correct type for the object patterns, but it has also
stored the correct type in the kernel representation.
Fixes#51795.
Bug: https://github.com/dart-lang/sdk/issues/51795
Change-Id: I73ce43e440db50d9942deb4a1eb4ee68a5c23142
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290900
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: I5e208edc53543de8d11aa6e81072ad7f61a23d52
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290913
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This code handles simple cases. Still TBD:
- Object patterns that refer to typedefs
- Object patterns that refer to classes with f-bounded polymorphism
- Inference of `dynamic` when there is no bound
Bug: https://github.com/dart-lang/sdk/issues/51795
Change-Id: I00acae6ba111f7b170650cfeffbfd2aaf7f71e42
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290347
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Switch statements on ints aren't required to be exhaustive.
Change-Id: I85701b098cacf9d01241dab821ae4c70f04a0710
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289450
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
This changes the analyzer and CFE to use the real exhaustiveness
checking algorithm. The fallback algorithm is kept and will be
removed once we know that the real exhaustiveness algorithm sticks.
Change-Id: Ic9df92c1ca9f7dec4cbdfa138dc6ed39ef2d4df5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288703
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This removes the use of [SwitchInfo], [SwitchCaseInfo] and
[ExhaustivessInfo] to pass on information for exhaustiveness checking
and instead uses the AST nodes directly in the constant evaluator.
Change-Id: Ie8d72b03e38334d1be9857794c38ed142dba5783
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288402
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This moves the lowering of patterns in switch statement/expression,
pattern assignment, pattern variable declaration and if-case statement
to the constant evaluator.
Change-Id: Ic5810e96b26a74987c50fd71b306e41b59504e1f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288401
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This change improves the fallback exhaustiveness algorithm so that it
also takes advantage of the old, pre-patterns exhaustiveness logic for
enums. This ensures that the fallback exhaustiveness algorithm won't
seem like a regression to users who are used to the old pre-patterns
behaviour. This required extending the old algorithm to handle
new-style switch statements (which have a different representation
from classic switch statements in the CFE) as well as switch
expressions.
We also turn on the fallback exhaustiveness algorithm by default.
Unit tests can still disable it by temporarily setting the global
variable `useFallbackExhaustivenessAlgorithm` to `false`.
The hope is that this algorithm will be short-lived; we are just using
it so that if we decide to enable pattern support in the near future,
users will experience something that is sound. That is, it may
require `default` or `_` cases more often than is strictly necessary,
but it shouldn't ever allow a non-exhaustive switch in a place where
an exhaustive switch is required.
Change-Id: I4a8b7f996b109c4ee8832f286c3b3bf3b216fe8b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284840
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This adds support for record types in the exhaustiveness algorithm.
The original algorithm was based on that record pattern would
match fields on all types, but that is no longer the case. Instead
record patterns only match corresponding to their own type. For this
reason the testing code is updated to create record spaces in relation
to a type. For instance, when the test create a record space {x: B}, it
is know create in relation to a type, say (x: A, y: A), and the create
space will therefore have (x: *, y: *) structure where the y: component
is implicitly Top, similar to how object patterns are used.
Unlike the Dart record types used for type checking and inference, the
record types used for exhaustiveness do not take the field types into
account for its subtype relation. This is avoid conclusions like
(int i, Object o) and (Object o, int i) having no values in common because
their corresponding types (int, Object) and (Object, int) are not subtypes
of each other. Instead, the subtype relation for record types used for
exhaustiveness only use the structure of the record to determine whether
two types are related.
Note though, that fields of a record type still know the type of the field.
This is used when expanded a record type into a space; the field spaces
will be derived from the field types in this case.
Change-Id: I84735d827494bcf384fd5f419d71933830ff5d15
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283182
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This allows the parser to parse constant patterns at lower precedence
level in order to recognize more expressions in this context. To
support this, _parsePrecedenceExpressionLoop special cases a few cases
that should _not_ be parsed as expression in a constant pattern context.
Closes#50996
Change-Id: I43bb0ce52d366bd2dfcf47e12eec5883402f668a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282100
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
