Previously, if multiple switch cases shared a body, and at least one
of those cases promoted a match variable using a `when` clause, the
promotion would not be carried over to the merged variable seen in the
shared case body. This was happening because promotions performed in
the `when` clause were applied to the promotion key associated with
the client's representation of the variable, whereas flow analysis was
computing the state of the merged variable based on the promotion keys
used internally while visiting the pattern (which don't include
promotions from the `when` clause).
With this change, flow analysis now computes the merged variable state
based on the promotion keys associated with the client's
representation of the variable components, so promotions from `when`
clauses take effect.
In the process, I've moved a lot of the tracking of these promotion
keys from the `TypeAnalyzer` class to the `_FlowAnalysisImpl` class.
This avoids the need to expose more flow analysis internals through
its public API.
Fixes#51399.
Bug: https://github.com/dart-lang/sdk/issues/51399
Change-Id: I8f1bb6e49ceb8441bb743c63af61c119df9041f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283441
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@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 optional error fires if patterns are enabled and the type
analyzer encounters a switch that is required to be exhaustive (i.e. a
switch expression, or a switch statement with an always-exhaustive
type), and flow analysis cannot prove that the switch is exhaustive.
This is intended to be available as a temporary workaround if we
decide to ship an early beta of the "patterns" feature before
exhaustiveness checking is sufficiently ready. We won't enable it
unless we need to, and we won't ship the final patterns feature with
it enabled.
Change-Id: I8c4c65c21abdc32b91537b3f30e7fd86f446f571
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283060
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This makes the unit tests clearer, since it's possible to tell the
meaning of each argument. It also simplifies the underlying unit
testing logic that formats the errors.
Change-Id: I7212fe0c909ca0b627f8a8889621af3a62a63f00
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283132
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The `numMergedCases` argument was a holdover from the old analyzer
representation of switch statements (in which cases sharing a body
were not merged); it is no longer useful.
Also, the analyzer tests in `switch_case_completes_normally_test.dart`
have been adjusted so that they no longer expect this error to be
issued when pattern support is enabled; this is consistent with the
spec (which removes this error).
Change-Id: Ia22a35b7a94ee585e0614d85012ee80b9dbc12d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283125
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This replaces the StaticTypeImpl with an implementation of the shared
exhaustiveness classes for a test type system.
This will ensure that the unittests test the shared part of the code
and make it easier to handle for instance fields on Object and
non-interface types.
Change-Id: I101cdd41cf86b2847bd5959c15d4138640e34679
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282882
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
For each subpattern of a record pattern, we define its "demonstrated
type" to be the type that the matched value has been promoted to if
the subpattern match succeeds. At the conclusion of visiting a record
pattern, we promote the whole record pattern's matched value to a
record type formed by combining together the demonstrated types of the
subpatterns. So, for example, the pattern `(int _, String _)`
promotes the matched value to `(int, String)`.
This change contains a lot of specific tests, to make sure the
demonstrated type of each kind of pattern behaves as expected; but the
actual machinery is general, since it just takes advantage of the type
promotion performed by the subpatterns.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: Ie4da81964b5ade657c23a598ee18982b793fb4ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282806
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
to be reachable for `ifTrue` and `ifFalse`.
This is because checking only the type to determine the reachability is not correct, since for the list and map patters, the expression can match, but this is not always true.
Fixes#51353
Change-Id: Iea53fd192d16e786fa8f2540425fc3cb97664d10
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282580
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, we kept track of the promotion information for a matched
value using a `ReferenceWithType` stored in the `_PatternContext`.
This information did not get updated in the event of a promotion, so
that meant that if a pattern tried to promote the same value twice
(e.g. `int _ && num _`), the second promotion attempt would not see
the effects of the first attempt, so it might wind up demoting the
matched value.
The solution is to just track the promotion key in `_PatternContext`,
and resynthesize the `ReferenceWithType` object (with the proper type)
whenever we need it.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I026a4d2f42875a003ce8840dfc88d65484bb33ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282800
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, we stored an EqualityInfo object representing the
scrutinee at the time of entry to the top level of the pattern; then
when handling, for example, a relational pattern using `==`, we used
that EqualityInfo object to decide how to handle the equality check,
*even if we were no longer at the top level of the pattern*. As a
result, we would sometimes come to incorrect conclusions about
relational patterns inside subpatterns. For example, we would fail to
see that `if ((null,) case (== null,))` is a guaranteed match, because
when processing the `== null` pattern, we would erroneously use the
EqualityInfo for `(null,)` (which is *not* `null`).
To solve this, we only store the scrutinee *reference* at the top
level of the pattern (this is sufficient to allow us to decide whether
or not to promote the scrutinee). When we encounter a pattern like
`== null`, we compute the appropriate EqualityInfo directly based on
the ExpressionInfo, type, and reference that are appropriate to the
current pattern level.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I4d1bc34fcb2d238e7b69e1b88df50b389410963c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282162
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
If a cast pattern fails to match, it throws an exception, so the
"match failure" code path should be unreachable.
To fix this, I added an optional argument `updateUnmatched` to
`patternRequiredType` to allow the type analyzer to tell flow analysis
whether a required type is enforced via a match failure or an
exception. I also renamed `patternRequiredType` to
`promoteForPattern`; this is in anticipation of an upcoming CL which
will use this method to promote to a type which isn't necessarily the
same as the required type.
I also discovered a few testing gaps while double-checking that every
call to `promoteForPattern` makes the proper choice about what to pass
for `updateMatched`; I've added test cases to cover these gaps.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: Id6f63138d01b481e7c9442469ce45e2aaa507a5a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281740
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
There are two new API methods: `equalityRelationalPattern_end` (for
relational patterns using `==` or `!=`, where some degree of flow
analysis is warranted) and `nonEqualityRelationalPattern_end` (for all
other relational patterns, where all we care about is making sure that
both the "matched" and "unmatched" branches are reachable).
I've moved most of the logic for constant patterns into
`equalityRelationalPattern_end`, since it's essentially the same logic
(except that `equalityRelationalPattern_end` also has support for
`!=`). So now, `constantPattern_end` simply calls
`equalityRelationalPattern_end` in the case where pattern support is
enabled.
I also had to make a change to `RelationalOperatorResolution` to make
it possible to distinguish `==` from `!=`.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: Idc5dbcfb02e3f8b57cfcccb3f46364fe34268ded
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280900
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Since matching a constant pattern implicitly performs an equality
check, we do the same flow analysis for constant patterns that we
would have done for an explicit equality test. There are only two
user-visible behaviours:
- If the constant pattern is a `null` literal, then in the code path
where the pattern fails to match, the scrutinee is promoted to a
non-nullable.
- If the constant pattern and the scrutinee are both `null` literals,
the pattern match is known to always succeed (so the code path where
the pattern fails to match is marked as unreachable).
The first of these two behaviours is genuine useful to the user, since
it allows things like:
switch (expr) {
case null:
...
...
default:
// expr is known to be non-`null`.
}
The second behaviour is not so useful, but seemed worth doing to
maximize code sharing and to keep the behaviour consistent between
pattern matching and explicit null checks.
Making this work required extracting some of the logic that was
formerly in `_FlowAnalysisImpl.equalityOperation_end` into a method
`_equalityCheck`, which understands how to analyze an equality check
regardless of whether it's an explicit equality expression or an
implicit part of a pattern. It returns an `_EqualityCheckResult`,
which `equalityOperation_end` examines to determine exactly what needs
to be done to for an equality test in an expression context;
similarly, `_FlowAnalysisImpl.constantPattern_end` now calls
`_equalityCheck` and then does the right thing for patterns.
It was also necessary to extract the logic from
`nullCheckOrAssertPattern_begin` for handling null checks in patterns;
that logic is now in `_nullCheckPattern`, which is also called by
`constantPattern_end`.
In the process of testing this change, I discovered (and fixed) a
minor bug: when analyzing a switch statement or switch expression, we
were re-capturing the value of the scrutinee before visiting each
pattern. This meant, for example, that we would analyze the following
code incorrectly:
int? i = ...;
switch (i) {
case _ when f(i = ...):
...
case null:
...
default:
// `i` should *not* be promoted to non-null here, because it's
// not guaranteed to be the same as the value matched by `case
// null` above.
}
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I4d30d6bc2673d9968e69f3384a37e1b2b9cc4a8f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280861
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, we treated cast patterns as a simple type-check, relying
on promotion machinery to supply the correct matched value type for
the inner pattern. In other words `P as T` was analyzed like `T() &&
P`. This had two unforunate consequences:
(1) If the type `T` was not a subtype of the matched value type, no
promotion occurred. So for example, if `x` had type `int?`, then
`if (x case var y as String?)` caused `y` to get an inferred type
of `int?` (which is clearly not what the user wants).
(2) Any promotions triggered inside the inner pattern would propagate
to the outer pattern. So for example, if `x` had the type
`Object?`, then `if (x case int _ as num)` caused `x` to be
promoted first to `num` and then to `int`. Although this was
sound, it seemed to me that it was strange and unexpected
behaviour, because the typical user expectation when performing a
cast is that only the type being cast to should be used for
promotion.
The solution to both of these is simple: we analyze the inner pattern
as though its matched value is distinct from the matched value
supplied to the cast, and has a static type of the cast type. Even
though in reality we know that the two values are the same, it's sound
for flow analysis to ignore that information.
Fixing this made it possible to add tests of some other patterns flow
analysis behaviours that weren't previously testable, so I've included
several tests in this CL that aren't strictly testing the change.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I8cf864c3bf20f55406cfd74aeb8891d8e81f93e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280207
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
There are two ways type promotion can occur when switch cases share a body:
(1) the scrutinee variable (if any) might be promoted, e.g.:
f(Object x) {
switch (x) {
case int _ && < 0:
case int _ && > 10:
// `x` is promoted to `int` because both cases promote the
// scrutinee variable to `int`.
}
}
(2) explicitly matched variables might be promoted at the time of the
match, e.g.:
f<T>(T t) {
if (t is int) {
switch (t) {
case var x && < 0:
case var x && > 10:
// `x` has type `T` but is promoted to `T&int`, because
// both declarations of `x` are in a context where the
// matched value has type `T&int`.
}
}
}
The existing flow analysis logic handles case (1) without any extra
work, because those promotions are joined as a natural consequence of
the flow control join at the end of matching the cases.
However, flow analysis has to do some extra work for case (2), because
the two copies of variable `x` are associated with different variable
declarations (and hence have different promotion keys). To ensure
that the promotions are joined in this case, we need to copy the flow
model for the two copies of `x` into a common promotion key prior to
doing the flow control join.
The bookkeeping necessary to figure out a common promotion key is
similar to the bookkeeping for logical-or patterns.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I9ee4ec5d797dae28099aafbaf34fbbeeee5cd626
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280201
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
There are three ways type promotion can occur in an or-pattern:
(1) the scrutinee variable (if any) might be promoted, e.g.:
f(Object x) {
if (x case int _ && < 0 || int _ && > 10) {
// `x` is promoted to `int` because both sides of the `||`
// promote the scrutinee variable to `int`.
}
}
(2) the implicit temporary variable that holds the matched value might
be promoted, e.g.:
f(Object Function() g) {
if (g() case (int _ && < 0 || int _ && > 10) && (var x)) {
// `x` has type `int` because both sides of the `||` promote
// the matched value to `int`.
}
}
For this sort of promotion to work, we need to
(3) explicitly matched variables might be promoted at the time of the
match, e.g.:
f<T>(T t) {
if (t is int) {
if (t case var x && < 0 || var x && > 10) {
// `x` has type `T` but is promoted to `T&int`, because both
// declarations of `x` are in a context where the matched
// value has type `T&int`.
}
}
}
The existing flow analysis logic handles cases (1) and (2) without any
extra work, because those promotions are joined as a natural
consequence of the flow control join at the end of matching the
logical-or pattern.
However, flow analysis has to do some extra work for case (3), because
the two copies of variable `x` are associated with different variable
declarations (and hence have different promotion keys). To ensure
that the promotions are joined in this case, we need to copy the flow
model for the two copies of `x` into a common promotion key prior to
doing the flow control join.
The bookkeeping necessary to figure out a common promotion key is
similar to the bookkeeping necessary to track the association between
the individual declared variable patterns and the joined pattern
variable (and this is bookkeeping that flow analysis is already
doing). So as part of this change I went ahead and removed the
`getJoinedVariableComponents` method (which was previously used by
flow analysis to query this association). This reduces the
constraints on the analyzer and CFE implementations by not requiring
them to do this bookkeeping themselves.
In the process I've made two additional small changes:
- I modified the logic for assigning types and finality to joined
variables so that if there is a finality conflict but no type
conflict, the common type is used; conversely, if there is a type
conflict but no finality conflict, the common finality is used.
This should help reduce follow-on errors.
- I added logic to ensure that if a variable is only declared on one
side or the other of a logical-or, flow analysis still considers
that variable to be definitely assigned. This should help reduce
follow-on errors.
Change-Id: I62f17adb6a51a583707c216ed48d941d1c621eea
Bug: https://github.com/dart-lang/sdk/issues/50419
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279756
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
The tables of subtypes and "factor" results used in testing the shared
analysis logic were becoming unwieldy and difficult to maintain.
Replace them with implementations of the subtype and factor algorithms
from the spec.
Change-Id: Ic607c3fda45a69661e094292a38a7bc8fd22859a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279748
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is consistent with the analyzer implementation and more intuitive
to work with.
In a follow-up CL I'll be adding logic that performs additional
manipulations on record types, and this change will make that CL
easier.
Change-Id: I4a0592e240a98f62d3730068600a84adeddfe09d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279656
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
A variable or wildcard pattern should only promote the scrutinee if
the expression is being matched is the scrutinee. If the variable
pattern is a subpattern, no promotion should occur.
E.g. promotion occurs here:
f(Object? x) {
if (x case int _) {
// `x` is promoted to `int`
}
}
But not here:
f(Object? x) {
if (x case num(sign: int _)) {
// `x.sign` is known to be an `int`, but `x` is simply a `num`.
}
}
Change-Id: Iaa5bab8ce5b81db9d5496ac0bf2ee10473302371
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279641
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This adds the initial implementation of exhaustiveness checking in
the analyzer and CFE. The checking is currently only performed in
switch statements and only handle a subset of the patterns.
Change-Id: Ia0050c2c80fbefe3e22615599136f9d919ebe4ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279173
Reviewed-by: Jonas Termansen <sortie@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Rather than allocate instances of PatternVariableJoin during the
"previsit" stage, the test is responsible for allocating them prior to
calling `run`. This will allow the test to include assertions in the
data structure passed to `run` that interrogate the state of the
joined variables (e.g. to check that they're properly promoted).
Change-Id: I7524c540872be153eb0a4cdf8bff679843335231
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279277
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This change is part of a process of shifting the responsibility of
tracking the set of variables defined by a pattern into the shared
analysis logic. A `variableName` parameter is added to
`analyzeDeclaredVariablePattern`, and a field `componentVariables` is
added to `MatchContext` to track the set of variables associated with
each variable name in a pattern. In the CLs that follow, I plan to
use this information in the place of the `getJoinedVariableComponents`
method. This will give the client more flexibility in deciding when
and how to join variables in logical-or patterns. (This is needed
because the CFE joins variables eagerly as a necessary part of
lowering, whereas the analyzer joins variables only at the end of the
pattern, to improve the quality of error messages).
Also, a `variables` parameter is added to `analyzeIfCaseElement` (to
make it consistent with `analyzeIfCaseStatement`), and the
`patternVariables` parameter is removed from `analyzePatternForIn`.
With these changes, we now consistently have a `variables` map in
situations where the pattern match is refutable (and thus logical-or
patterns are allowed), and we have no `variables` map in situations
where the pattern match is irrefutable (and thus no variable joining
is needed).
Change-Id: I1c511d12e827c52ef9bd8b7ff2b561ea9713a932
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279239
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
In order to handle variable patterns inside logical-or patterns, flow
analysis will need to model the implicit temporary variables that
represent the variables before they are joined to form the final
variable value. This CL adds the necessary logic to model this:
- `declaredVariablePattern` is now responsible for initializing the
temporary variable (the client no longer needs to call `initialize`
when analyzing a variable pattern). It returns an integer
representing the implicit temporary variable.
- A new API call, `assignMatchedPatternVariable` can be used by the
client to transfer the variable from the implicit temporary variable
to a user-accessible variable. For now, the shared analysis logic
always calls this from `analyzeDeclaredVariablePattern`, after
calling `declaredVariablePattern`. However, in the future, it will
postpone the call until after these temporary variables are
implicitly joined by logical-or patterns.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I2b78a46c11d0d46c8e0a8691c2a2ce49dceb2a24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279078
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This adds support for checking the stack content against a stack base,
which enables testing for an empty stack, relative to the stack base.
Test added for the stack checker.
Change-Id: I4937a4c77c0c2d8d6673e7848ccedfd8b1491ab6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279340
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
In a future CL I will need the ability for flow analysis to initialize
promotion keys that aren't associated with any particular variable.
In anticipation of that, this CL refactors `FlowAnalysis.initialize`
so that it immediately looks up the unpromoted type of the variable
(which is the only information it needs), and thereafter just uses the
variable's promotion key.
From the point of view of flow analysis clients, there is no
functional change.
Change-Id: I54794bde49c7af745b43a09914f70c9c4e6d48da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279074
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Previously, if a switch case was unreachable, we would not insert a
synthetic break at the end of it. Technically this is not a problem
(since a synthetic break is only required to prevent one case from
falling through to another at runtime, and unreachable code is by
definition never reached at runtime). However, it makes it confusing
for CFE and back-end developers, and there's little harm in adding the
synthetic break anyway.
So with this change, we determine whether to add the synthetic break
by checking whether the bottom of the case block is reachable *from
its top* (rather than globally reachable).
Fixes#50994.
Bug: https://github.com/dart-lang/sdk/issues/50994
Change-Id: I17757b182c29da782457adc057b1b8a6fc91e55e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278897
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Paul Berry