It's not necessary to call `FlowAnalysis.declare` from
`TypeAnalyzer.analyzeDeclaredVariablePattern` because the call that
immediately follows, `FlowAnalysis.assignMatchedPatternVariable`,
completely overwrites the variable's flow model.
Change-Id: I8293672c7fefec7eddfb4d43c38d81d8dcbe9aea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280202
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>
In AOT compiled programs we may not know whether a String is one-byte or
two-byte (internal or external) and as a result, calls to .codeUnit()
are not inlined.
The string canonicalizer uses it for two purposes: creating hashes for
substrings and comparing substrings.
=> The ladder we can replace by a call to .startsWith() with a specific
offset.
Change-Id: I31025b3be03277a9c9505ef09d9c8a41a57e7839
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279974
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Martin Kustermann <kustermann@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>
Right now the analyzer clears out the string canonicalization cache
after each file parse.
Similarly when the analyer serializes various information into
*.{unlinked2,linked,resolved} files & deserializes them again, it will
only have canonicalized strings per unit.
This means that strings that are common across compilation units will
exist many times in the heap, thereby increasing memory consumption of
the process.
This CL tries to avoid that by
* Using one string canonicalization cache across CFE & Analyzer
* Making the string canonicalization cache remember how often an entry
was used
* Pruning the cache instead of clearing it: We keep the most frequently
used elements while maintaining a cache that is <= 5 MB.
* Change analyzer code to use the string canonicalization in various
places to avoid many duplicate strings in the heap.
When running analyzer on flutter (and it's transitive code), the
analyzer itself has a heap of around 610 MB.
This CL reduces the memory consumed by strings from ~43 MB to ~33 MB
(~ 20 MB are alone dart source files which are unique). We do this at
the expensive of maintaining the string cache, which costs around 2 MB.
That leads to 8 MB of savings.
TEST=ci
Change-Id: Ic28d70492ab0d376e10714c7ecf2c258c790a022
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255245
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@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>
This moves the ValueKind and NullValue interface to the util package
and add the NullValue as an interface to the NullValue enum, which has
now been renamed to NullValues.
This prepares for reusing the stack checking mechanism from the
StackListener in the inference visitor of the CFE. The adding of
NullValue as an interface, allows for using ValueKind and NullValue
that are specific to the types used in the stack.
Change-Id: I7d6b3d3932753898d87ef774a95460832efdc969
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279084
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Generative synthetic constructors of @staticInterop classes are already
disallowed, but this adds errors for tear-offs of such members as well.
This also disallows tear-offs of @anonymous @staticInterop factory
tear-offs. This aligns with what we want to do with object literal
constructors going forward, as tear-offs will implicitly have different
semantics than direct invocations. To avoid that inconsistency, we
disallow tear-offs here.
Change-Id: Ifc9e4a9251743613ee1ea2eca6e42e36c3b20461
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278645
Reviewed-by: Riley Porter <rileyporter@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>
Johnni Winther