Support for `when` clauses requires flow analysis integration, so that
`when` clauses can promote variables, e.g.:
f(int x, String? y) {
switch (x) {
case 0 when y != null:
// y is known to be non-null here
}
}
Support for labels in switch statements had a small flaw: we weren't
reporting an error in the case where a label shared a case body with a
pattern that tried to bind a variable, e.g.:
f(int x) {
switch (x) {
L: // Error: does not mind the variable `y`
case var y:
...
}
}
Change-Id: I0b2bb4721a6b3a8f7898df682b24b75ddb6e44ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256605
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This change introduces the TypeAnalyzer methods
analyzeConstOrLiteralPattern, analyzeExpression,
analyzeInitializedVariableDeclaration, analyzeSwitchExpression,
analyzeSwitchStatement, analyzeUninitializedVariableDeclaration, and
analyzeVariablePattern. These are sufficient to analyze legacy switch
statements and legacy variable declarations, as well as switch
statements and switch expressions involving either constants or
variable patterns.
Although the code is not used in the analyzer or front end yet, it is
unit tested in isolation, and it's integrated into the existing flow
analysis unit tests.
A few minor tweaks had to be made to flow analysis to support this new
functionality. There should be no visible effect to existing analyzer
or front end behavior.
Change-Id: Ie8ec31ca92d5f2f7a7f6f6a20ca1baba3c6b28f9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256604
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change introduces the mixin TypeAnalyzer, which is intended to be
mixed into analyzer and front end classes to provide shared logic for
type analysis of Dart code. This work is currently experimental, and
not hooked up to the analyzer or front end, but the eventual hope is
that it can replace the logic that's currently duplicated between the
analyzer's ResolverVisitor and the front end's InferenceVisitorImpl.
A secondary goal of introducing this code is to allow some of the
static consequences of the patterns proposal to be explored now, even
before parser support is finished.
To avoid introducing additional code duplication while the project is
in this experimental phase, I'm going to attempt to restrict myself as
much as possible to prototyping functionality that is not yet
implemented in the analyzer or front end. This initial CL is an
exception; it introduces the method `analyzeIntLiteral`, which
duplicates logic that already exists in both the front end and
analyzer that analyzes integer literals. I'm doing this because it's
just complex enough to serve as a validation of the basic approach,
and because integer literals will be handy in writing test cases for
the expanded switch functionality in the patterns proposal, which I
plan to work on next.
Although the code is not used in the analyzer or front end yet, it is
unit tested in isolation, and it's integrated into the existing flow
analysis unit tests.
Change-Id: I07c7cd709eec9e8492669f2dc8db57fb7c10798f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255081
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is a preparatory step towards sharing type inference logic
between the front end and analyzer. The ExpressionTypeAnalysisResult
interface will be returned from the shared `analyze` methods for
expressions, as a container for all the information needed by both the
client (front end or analyzer) and by the code that analyzes the
containing expression or statement.
For now, the only implementation of the interface is
SimpleTypeAnalysisResult, which represents the result of analyzing a
simple expression with no null shorting. In future CLs I plan to add
more types, recording information such as:
- For an integer literal, whether it was implicitly converted to
`double`.
- For a binary operator or a compound assignment, the resolved binary
operator.
- Information necessary to coordinate null shorting.
And so on. There is a placeholder method `resolveShorting` that will
encapsulate null shorting logic, but it doesn't do anything special in
the case of SimpleTypeAnalysisResult.
At the moment, only test code is affected.
Note that this approach isn't free; every time we analyze an
expression we wrap the resulting type in a SimpleTypeAnalysisResult,
so there's a cost of one allocation per expression in the user's
program. I don't believe this will be a problem in practice, because
the front end uses a similar approach, so it is already paying this
penalty.
Change-Id: Ic407089b6eb9e717c65d73766e7c836e161ef0da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255080
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
- Variable types are no longer specified in the call to the `Var`
constructor; they are now specified in the call to `declare`. This
paves the way for supporting variable pattern syntax, in which a
single variable might appear in multiple variable patterns, and have
its type specified in each pattern. The properties `isFinal` and
`isLate` are also moved to `declare` for consistency.
- Variables with inferred types are now specified by simply not
including a type in `declare`; it's no longer necessary to specify
`isImplicitlyTyped: true`.
- `declare` now supports an `expectInferredType` argument to allow the
inferred type of an implicitly typed variable to be tested.
- The tests now check that variables are assigned a type before flow
analysis requests it; previously this was not tested, and the flow
analysis tests sometimes did things in the wrong order. (The
analyzer and CFE have always done this in the proper order though).
- The tests now support some of the crazy types that arise during type
parameter promotion, e.g. they can now distinguish `(T&int)?` from
`T&(int?)`.
- Flow analysis tests now properly replicate the analyzer and CFE
behaviors for converting the static type of an initializer
expression to the corresponding inferred variable type: (a) `Null`
is converted to `dynamic`, and (b) type parameter promotions are
dropped.
Note that this last behavior (dropping type parameter promotions) has
a lot of subtleties, and I'm not convinced the CFE and analyzer do it
soundly in all cases (I've already found one such soundness bug:
https://github.com/dart-lang/sdk/issues/49691). In a later CL, I plan
to add a more thorough set of language tests to verify that we don't
have other lurking soundness issues.
Change-Id: I6f2cd20db1f07b34e0ad4e7002351c8de846b125
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255600
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Several unusual constructs that lead to unreachable code are now
recognized by flow analysis:
- Control flow after an expression of the form `e ?? other` or `e ??=
other`, where `e` has static type `Null` and `other` has static type
`Never`, is considered unreachable.
- Control flow predicated on an expression of the form `e is Never`
evaluating to `true` is considered unreachable.
- Control flow predicated on an expression of the form `e is! Never`
evaluating to `false` is considered unreachable.
- Control flow on the RHS of a null-aware access such as
`e?.property...`, `e?.property = ...` or `e?.method(...)`, where `e`
has static type `Null`, is considered unreachable (Note: this can
arise in the presence of extension methods).
Previously, these behaviors only took effect if `e` was a reference to
a local variable.
Note: the change to `regress/issue_31180` is because I’ve corrected
the behavior of implicit temporary variables to not undergo a type
change from `Null` to `dynamic`, so the dead code part of `null?[1]`
is now erroneous. (I had to make this change in order for the last
bullet above to work properly; without it, the type change to
`dynamic` prevents flow analysis from recognizing that the code to the
right of `?.` is unreachable.) There's no behavioral change to
correct code, but I've captured the behavioral change to incorrect
code in
`tests/language_2/null_aware/null_aware_index_on_null_error_test.dart`.
Bug: https://github.com/dart-lang/sdk/issues/49635
Change-Id: I8b24b3b040a34f897c0b61dcb9bd105be6d0af6d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251280
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
A get of a promotable field on a non-promotable field shouldn't
promote, otherwise this code would be unsound:
class C {
D get _field1 => D();
}
class D {
final int? _field2 = randomBool() ? 1 : null;
}
main() {
var c = C();
if (c._field1._field2 != null) {
// Problem: `c._field1` returns a different `D` each time, so there's
// no guarantee that `c._field1._field2` is non-null the second time we
// access it!
print(c._field1._field2.isEven);
}
}
This change has no user-visible effect because field promotion is not
yet enabled.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I9a60cd343dff14cded03df700d7c2b62a251487a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255821
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This change breaks flow_analysis.dart into the following libraries:
- assigned_variables.dart (for the AssignedVariables class and related
code)
- promotion_key_store.dart (for the PromotionKeyStore class)
- type_operations.dart (for the TypeOperations mixin and related code)
- flow_analysis.dart (for the rest of flow analysis)
And it breaks mini_ast.dart into the following libraries:
- flow_analysis_mini_ast.dart (functionality specifically concerned
with testing flow analysis)
- mini_ast.dart (functionality not specifically related to flow
analysis)
This is in preparation for trying to share some more type inference
behaviors between the analyzer and CFE.
Note that although the diff is big, the only changes in this CL are
moving code from one place to another, renaming some class members
from private to public, and updating imports.
Change-Id: I71768f03b1e75ed754c7b7af39f6cf7f03c4fe44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254462
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change splits the `Harness` class into a base class, `Harness`,
which in principle can be used for testing type inference logic in
general, and a derived class `FlowAnalysisTestHarness`, which is
specialized for flow analysis tests.
This is in preparation for trying to share some more type inference
behaviors between the analyzer and CFE.
Change-Id: Ic56b8dd8748065ca59e246e0d804946cc69203c3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254280
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This improves separation of concerns, since the `PromotionKeyStore`
class isn't really concerned with what fields are promotable; it's
just a mechanism for assigning unique integer identifiers to
promotable things.
I've moved `_promotableFields` to the main `_FlowAnalysisImpl` class.
To avoid having to add it as a separate argument to a lot of
[FlowModel] methods, I've created a new [FlowModelHelper] interface
that provides these methods with access to several fields in
`_FlowAnalysisImpl`.
Change-Id: I0280d0c0b95714521afbe68a07e7b3b54f23b7df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254003
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
In a future CL, this will allow me to move flow_analysis_test.dart's
global variable _promotionKeyStore into the testing harness, which is
a prerequisite for some clean-up work I'm doing on flow analysis.
Change-Id: I44943b2706e237213896164910b4c6f488aa2794
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253901
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, flow analysis used the class `ReferenceWithType` to track
references for which it knew the type, and `Reference` (and its
subclasses) to track references for which it didn't know the type (or
for which the type was unimportant).
This change removes the `Reference` class hierarchy, in favor of just
using the integer promotion keys. This should reduce the number of
memory allocations that flow analysis needs to make.
A few pieces of information previously maintained by the `Reference`
class hierarchy are now tracked elsewhere: the logic for computing
non-promotion reasons is now in
`_FlowAnalysisImpl._getNonPromotionReasons`, and the property name and
property member (previously maintained by `_PropertyGetReference`) is
now maintained by `_PropertyReferenceWithType` (a new subclass of
`ReferenceWithType`).
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I72f2d80b3256bf8b9c9a30bcc55666ecb7c31e47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250242
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Each variable is mapped to a unique integer "promotion key", which is
used as an index into the `FlowModel.variableInfo` map. This paves
the way for adding entries to `FlowModel.variableInfo` to represent
promoted properties. It also cleans up the previous hacky way we used
to use `null` as a map key to represent `this`.
In a future CL I plan to try to replace `FlowModel.variableInfo` with
a list rather than a map. This should improve both memory and CPU
usage.
As a side effect of this change, many classes related to flow analysis
no loger need a `Variable` type argument, including the `EqualityInfo`
class (which is exposed to clients).
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I853ca835c6b36ab9865bd187973c6524a7471db0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250120
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change moves the `variableType` method from the class
`TypeOperations` to a new class, `VariableOperations`, which in turn
allows removing the type parameter `Variable` parameter from
`TypeOperations`. A new class, `Operations`, is introduced to serve
the role served previously by `TypeOperations` for flow analysis
clients (i.e. it is the base class that clients should extend).
This paves the way for a future CL that will remove the type parameter
`Variable` from other classes inside flow analysis.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: Ic45d07a0f873b692fda4b6f807c1130ac592b010
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250108
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Move TypeDeclarationResolver to be available in the Declaration phase.
Add IntrospectableType interface, and IntrospectableClassDeclaration which implements it (we will eventually have IntrospectableMixinDeclaration, IntrospectableEnumDeclaration, etc).
Migrate ClassIntrospector to InterfaceIntrospector, which operates on IntrospectableType instances instead of ClassDeclaration instances.
Question: Possibly `InterfaceIntrospector` should have a different name, maybe just `TypeDeclarationIntrospector`?
Change-Id: Ifd202bad61eeae5f7d76d769d9d96a866c0fecdb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247060
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
In theory, the order of resolving function literals during a type
inference stage shouldn't matter, because the only effect of visiting
a function literal during type inference is to capture writes, and
captured writes don't affect flow analysis in other function literals
due to the fact that a variable that's write captured anywhere is
already unpromotable in other closures.
However, to be on the safe side and to make the resolution process
deterministic, it seems better to resolve the function literals in
source order during each stage.
See https://github.com/dart-lang/language/issues/731 (improved
inference for fold etc.)
Change-Id: Iaecdfbd7e1eb89583a7d744371c40e2b904c7634
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241020
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
As Lasse pointed out in an internal review of the fix for
https://github.com/dart-lang/language/issues/731 (improved inference
for fold etc.), what I've implemented actually doesn't apply to all
closures, just to function literals. (Tearoffs of local functions are
also closures). This change adjusts the nomenclature to consistently
use the term "function literal" rather than "closure".
Change-Id: I64f255955494f7f881a081c32c9220c41cdc7990
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240861
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change addresses a corner case discovered during internal testing
of the fix for https://github.com/dart-lang/language/issues/731
(improved inference for fold etc.): if there is no order dependency
forcing us to do a round of horizontal inference between visiting
non-closure arguments and closure arguments, then it's important that
we *don't* do a round of horizontal inference before visiting the
closure, because there is a risk of inferring too narrow a type.
The new algorithm includes all the invocation arguments in dependency
analysis, and the dependency rules are structured such that
non-closure arguments always wind up in stage 1. If there is no
dependency between non-closure arguments and closure arguments, then
the closure arguments also wind up in stage 1, and no horizontal
inference occurs. If there is a dependency, then closure arguments
wind up in stage 2 or later, and horizontal inference occurs between
stages.
Change-Id: Ida0b28da211f63191c9c9c39e6004893617507bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240442
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
- removes the `loadMacro` api entirely
- adds new apis to MultiMacroExecutor
- registerExecutorFactory
- unregisterExecutorFactory
- libraryIsRegistered
- Removes the MacroClassIdentifier, instantiateMacro takes a library uri and class name
- Drop precompiledMacroUris from CompilerOptions, only have a macroExecutor now
Change-Id: Ic33933b34dbfd637da3d841ff496582f688dcd87
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239466
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
This algorithm will form a piece of the solution to
https://github.com/dart-lang/language/issues/731 (improved inference
for fold etc.). In particular, when performing generic type inference
on an invocation where more than one argument is a closure, it will
determine the order in which type inference should visit the closures
to maximize the chances of producing meaningful assignments of types
to type parameters.
Change-Id: Ifc2202cb713965373981574f276dffbf1b3f48c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239364
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously this was the responsibility of the client, but not all
clients of `DependencyWalker` were doing this, causing potentially
duplicate work in the analyzer.
Change-Id: Ice68b371a75c290fbd6b144fc927b48a5dd87620
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239261
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
For https://github.com/dart-lang/language/issues/731 (improved
inference for fold etc.), I want to re-use this logic as part of the
algorithm for computing the order in which to visit an invocation's
closure arguments. That algorithm, in turn, will be shared between
the front end and the analyzer, so the DependencyWalker needs to move
into _fe_analyzer_shared first.
Change-Id: I953e027448cbd660dc30f92d1bdcce26a8d7bda3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/238920
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This adds support for generating truly unique fresh names when building
augmentation library code. The synthesized names are used for import
prefixes and as placeholders for not-yet-inferred omitted type
annotations.
Change-Id: I6fe9bc37b39a243c93187f561ee32e07237341f1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237823
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This implements TypeDeclarationResolver for the CFE. Adds a test to
the macro api test, which includes support for throwing and catching
the ArgumentError required by the api.
Change-Id: I5bb7948064ba5a2c215289915fc10314fd28aa1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237020
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This adds support for [interfacesOf] and [mixinsOf] of the
[ClassIntrospector] interface. To support this, the mixed in
[ClassHierarchyNode] is now included in the [ClassHierarchyNode] for
a mixin application.
Change-Id: I30cef4b462b1b2b52acf1bd286d82ebc1c5caa4e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/236883
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This also:
- Adds support to the bootstrap code to handle communicating via sendPort or stdio
- Refactors pretty heavily the isolate executor code such that it can be reused.
- Refactors the isolated executor test into a shared test for all executors
- Removes the mirror based executor entirely - I don't see a path forward with this one.
- Reorganizes all executor based files under macros/executor
Change-Id: I2fc6bb3e6ce56c8b331719f4dc08617b3a521398
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233200
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
- Support TransferableTypedData in the bootstrap code and IsolatedExecutor.
- Also removed the fake executor and conditional import. Mode will likely be chosen based on factors other than just the availability of `dart:isolate`.
- Run the tests in both serialization modes.
Change-Id: I5c731d192c0d3a8cdc5f7fb900dc07a32f4f4d51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/232981
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
With the optimizations suggested this does now equal or outperform the JSON serializer in all scenarios, the latest numbers from the benchmark are as follows:
Isolate.spawn + SerializationMode.jsonClient: 0:00:00.041666
Isolate.spawnUri + SerializationMode.jsonClient: 0:00:00.069551
Separate process + SerializationMode.jsonClient: 0:00:00.177171
Isolate.spawn + SerializationMode.byteDataClient: 0:00:00.040990
Isolate.spawnUri + SerializationMode.byteDataClient: 0:00:00.059319
Separate process + SerializationMode.byteDataClient: 0:00:00.080008
Change-Id: If5431513c7487d8b7af350381e794cbd61c1be42
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/232027
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
This adds an initial test for implementation of the macro api that
can be shared between implementations.
The test consists of a set of macros and a library of declarations that
apply these macros. When applied the macros work as unittest tests that
check that the macro api give the expected results for the various
declarations.
Change-Id: Id4183e8ac90fa96ef1ebc01b1c9f15cc413b6443
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/232383
Auto-Submit: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Paul Berry