https://dart-review.googlesource.com/c/sdk/+/498840 added support for
promotion of properties (private, final instance variables with a name
which isn't used much for other purposes) in the context of anonymous
methods.
This CL reduces the set of situations where this feature is enabled such
that only `this` will allow property promotions to be carried in (such
that `this._x` is promoted in `v.=> this._x` when `v` is such that
`v._x` has been promoted before the anonymous method occurs). It also
generalizes the mechanism such that property promotions are carried out
(so we can do `if (v.=> _x is int) v._x.isEven;`).
Tests has been adjusted accordingly.
Change-Id: Ibe70713d3d9c89a6d95f9c3dd28df8f147cb518d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502660
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
This CL adds support for promotion of certain private final instance
variables along with anonymous methods. The promotions do not differ
from the ones which are already available in Dart without anonymous
methods, but it requires some generalizations to handle the changing
value of `this` which is made possible by anonymous methods.
Change-Id: I720a5fa6d29a8a7d19bb2e167dc135f97492b525
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498840
Reviewed-by: Paul Berry <paulberry@google.com>
This reverts commit 3eb697c0af.
Reason for revert: Internal Google3 breakages
Original change's description:
> [flow analysis] Fix unsound type promotion in inner async/generator functions.
>
> An `await` expression or `yield` statement suspends the current
> function and allows other code in the same isolate to execute. In the
> case of nested functions, an `await` or `yield` in the inner function
> can allow the outer function to continue executing. That means that if
> the inner function promotes a local variable belonging to the outer
> function, then it isn't sound to carry that promotion past an `await`
> or `yield`.
>
> This change fixes the unsoundness by adding a flow analysis method
> `suspension`, which the shared type analysis logic uses to tell flow
> analysis that an `await` or `yield` has been found. The `suspension`
> method un-does the promotions of any variables that might be written
> to while the inner function is suspended.
>
> Fixes https://github.com/dart-lang/sdk/issues/62889.
>
> Change-Id: I77eaf997159819a7c50f44b67174d2aa6a6a6964
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499382
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Commit-Queue: Paul Berry <paulberry@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> Reviewed-by: Bob Nystrom <rnystrom@google.com>
Change-Id: I187ba9a347394946ecc8749d35dc7914d271b90a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500540
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Paul Berry <paulberry@google.com>
An `await` expression or `yield` statement suspends the current
function and allows other code in the same isolate to execute. In the
case of nested functions, an `await` or `yield` in the inner function
can allow the outer function to continue executing. That means that if
the inner function promotes a local variable belonging to the outer
function, then it isn't sound to carry that promotion past an `await`
or `yield`.
This change fixes the unsoundness by adding a flow analysis method
`suspension`, which the shared type analysis logic uses to tell flow
analysis that an `await` or `yield` has been found. The `suspension`
method un-does the promotions of any variables that might be written
to while the inner function is suspended.
Fixes https://github.com/dart-lang/sdk/issues/62889.
Change-Id: I77eaf997159819a7c50f44b67174d2aa6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499382
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This makes the arguments to `ScannerConfiguration` required and uses the current enabled-by-default state as default.
Furthermore, an extension method is added for `ExperimentalFeatures` (and object already required by the parser) that creates the `ScannerConfiguration` based on the enabled features. This method is used throughout the analyzer and CFE, ensuring that the scanner is set up consistently with the parser.
Change-Id: Id51456d5a547b06d0925865e35e01d9dc7d65381
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493760
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Moves some of the business logic for type analyzing yield statements
from the analyzer and front_end codebases into the shared
`TypeAnalyzer` class. There is no functional change.
This paves the way for fixing
https://github.com/dart-lang/sdk/issues/62889 (Unsound type promotion
in inner async/generator functions), which will require the type
analysis of yield statements to be integrated more closely with flow
analysis. Sharing the type analysis logic will avoid the need to do
that integration twice.
Change-Id: I36c1f5e45e33d9e46dc934fb71bc634d6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491260
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Adds the class `StatementTypeAnalysisResult` to the shared type
inference codebase, along with a test method,
`checkStatementTypeAnalysisResult`, which will allow unit tests to
examine it. This class is the base class for the return type of all
statement analysis methods in the shared `TypeAnalyzer`
class. (Previously these methods returned void).
There are no fields in `StatementTypeAnalysisResult`; for now, it is
intended solely as a base class. To avoid incurring an extra runtime
cost, statement analysis methods that don't need to return any data
have been changed to return `const StatementTypeAnalysisResult()`
rather implicitly than returning `null`.
So far there are no derived classes. In a follow-up CL I will add an
`analyzeYieldStatement` method to the shared `TypeAnalyzer` class; it
will use a derived class to communicate information about the operand
to the front end.
Change-Id: I83c80b15d18e88fa80c08999bb78ca2f6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491240
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Moves the bulk of the business logic for type analyzing await
expressions from the analyzer and front_end codebases into the shared
`TypeAnalyzer` class. There is no functional change.
This paves the way for fixing
https://github.com/dart-lang/sdk/issues/62889 (Unsound type promotion
in inner async/generator functions), which will require the type
analysis of await expressions to be integrated more closely with flow
analysis. Sharing the type analysis logic will avoid the need to do
that integration twice.
Change-Id: I9f8770ce8127a960b746a89ee3e370ed6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489480
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
In the front end, the logic for determining when to report a
`voidExpression` error is integrated into the resolution pass. The
method `InferenceVisitorImpl.inferExpression` accepts a named
parameter `isVoidAllowed`; if the value is `false`, and the
expression's static type is `void`, an error will be reported. The
parameter is optional with a default value of `false`; this means that
the parameter only needs to be specified in the situations where
`void` is allowed; this aligns well with the language spec, which
lists the situations in which an expression may have type `void`, and
declares that in all other situations it is an error.
This change integrates the front end's `isVoidAllowed` boolean with
the shared type analysis logic in `package:_fe_analyzer_shared`, so
that the shared logic can specify when a subexpression should not be
void. This will pave the way for fixing
https://github.com/dart-lang/sdk/issues/62939 (Switch scrutinees of
type void are erroneously accepted by the front end). It also will
pave the way for sharing the logic for analyzing await expressions,
which will make it easier to fix
https://github.com/dart-lang/sdk/issues/62889 (Unsound type promotion
in inner async/generator functions).
Note that the analyzer doesn't use the `isVoidAllowed` parameter; it
has its own mechanism for detecting invalid uses of void (which is
more ad hoc). In the long run I would like to make the analyzer's
mechanism more like the front end's, for easier code sharing; see
https://github.com/dart-lang/sdk/issues/62942.
Change-Id: Ibb8dd072d6aff980d7df0b2e94f434ee6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489501
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Adds the unit test method
`ProtoExpression.checkExpressionTypeAnalysisResult`, which allows unit
tests of the shared analysis logic to access the object returned by
`TypeAnalyzer.dispatchExpression`.
This replaces ad-hoc logic that was used to test
`IntTypeAnalysisResult.convertedToDouble`, and allows testing of
`PatternAssignmentAnalysisResult.patternSchema` (which was previously
not covered by unit tests).
It also paves the way for shared analysis code I intend to introduce
for await expressions, as part of fixing
https://github.com/dart-lang/sdk/issues/62889.
(In principle, this mechanism could also be used to test the fields in
`SwitchExpressionResult`, but I believe I can refactor the code in
such a way that that class is no longer necessary, so I will try doing
that in a follow-up CL. If that doesn't work out, I'll add tests of
`SwitchExpressionResult` in a follow-up CL.)
Change-Id: I571c55f708b03e67ab491994675e53e16a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489460
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Prior to the introduction of the "anonymous methods" experiment, a
`return` statement and a `throw` expression behaved identically from
the point of view of flow analysis, since both had the effect of
causing control flow to jump outside the function that flow analysis
is analyzing*. So they were both implemented using a single flow
analysis method called `handleExit`.
(*Technically a `return` from an inner function could lead to a point
in an enclosing function, and a `throw` could lead to a `catch`, but
flow analysis handles both of these possibilities using a conservative
approximation (see the `FlowModel.conservativeJoin` method), rather
than modeling them as direct jumps.
But a `return` statement inside a block-bodied anonymous method is
known to jump directly to the code that follows the anonymous method
invocation, so `handleExit` is not the correct way to model it.
Prior to this CL, this was handled in the analyzer's resolver (the
corresponding CFE logic hasn't been written yet) by treating anonymous
methods as a kind of loop construct. When visiting a return statement,
the resolver would find the innermost enclosing function expression,
local function, or block-bodied anonymous method; if it was a
block-bodied anonymous method, then it would achieve the desired
effect by calling `FlowAnalysis.handleBreak` rather than
`FlowAnalysis.handleExit`. This was an abstraction leak, because in
effect it put some of the business logic of flow analysis in its
client (namely, the knowledge that return statements in block-bodied
anonymous methods have a different flow analysis behavior than return
statements elsewhere).
This CL moves this business logic into flow analysis through the
addition of a `FlowAnalysis.handleReturn` method.
Flow analysis keeps track of whether the current point in the code
being analyzed is inside a block-bodied anonymous method using the new
field `FlowAnalysis._anonymousBlockContext`, which points to either
`null` or an instance of a new type, `_AnonymousBlockContext`. This
field is updated in proper nesting fashion by the methods:
- `anonymousBlockBody_begin`
- `anonymousBlockBody_end`
- `_functionExpression_begin`
- `_functionExpression_end`
Finally, some aspects of
https://dart-review.googlesource.com/c/sdk/+/482786 that are no longer
necessary are rolled back:
- A node no longer needs to be passed to `anonymousBlockBody_begin`.
- The mapping from nodes to branch targets is changed back to a
mapping from statements to branch targets, since it no longer needs
to accept an anonymous method invocation as a key.
Change-Id: I8b0f35cab016fc5bd609cfa1581ecaa36a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/485020
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Removes the `_flowAnalysisInfoMap` field from the `Harness` class,
which serves as the test harness for the mini_ast used in flow
analysis testing, along with the methods `getFlowAnalysisInfo` and
`storeFlowAnalysisInfo` that did map lookups. Calls to
`getFlowAnalysisInfo` are replaced with logic that pulls the flow
analysis expression info directly from the expression analysis result,
and calls to `storeFlowAnalysisInfo` are dropped (since they are no
longer needed).
Also, the assertion is dropped from `dispatchExpression` that used to
verify that the information stored in the map matched the information
stored in the expression analysis result.
Change-Id: I6a6a69643e0345a1f5ef84f09340119b56bfed12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482580
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Makes the following updates to the mini_ast used for flow analysis
testing:
- When visiting a `Write` node, the flow analysis expression info is
pulled directly from the analysis result of the RHS and passed to
`_visitWrite`, rather than deferring to `_visitWrite` to look it up
using `getFlowAnalysisInfo`.
- When computing the promoted type or the promotion chain of a
`Property` node, the flow analysis expression info is pulled
directly from the analysis result of the target (by
`_computeMemberAndFlowAnalysisInfo`) rather than looking it up using
`getFlowAnalysisInfo`.
- When creating the guard variable for a null-aware property access or
method call, the result of analyzing the target is passed directly
to `createNullAwareGuard`, allowing it to obtain the flow analysis
expression info directly rather than fetching it from
`getFlowAnalysisInfo`. The modified expression info for the
null-checked target is then returned directly back to the caller
using another analysis result, rather than requiring the caller to
look it up using another call to `getFlowAnalysisInfo`.
These changes pave the way for removing the `getFlowAnalysisInfo` and
`storeFlowAnalysisInfo` methods from mini_ast, along with the map they
use for storage.
Change-Id: I6a6a6964d4533c16f622f08b302a7f62c959b58f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482562
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Migrates the mini_ast used for flow analysis testing so that instead
of associating expressions with expression info objects using
`FlowAnalysis.getExpressionInfo` and
`FlowAnalysis.storeExpressionInfo`, it does so using its own private
map, which it accesses using the new methods `getFlowAnalysisInfo` and
`storeFlowAnalysisInfo`.
This paves the way for two independent arcs of work:
- Removing the `getExpressionInfo` and `storeExpressionInfo` methods
from `FlowAnalysis`.
- Simplifying mini_ast so that it tracks expression info objects using
`ExpressionTypeAnalysisResult.flowAnalysisInfo` rather than its own
private map.
A similar effort is underway for the analyzer and front_end, which
should carry the following benefits:
- It will make type analysis more performant by avoiding map lookups
- It will decrease the risk of subtle bugs when one expression is
changed into another.
Change-Id: I6a6a6964f0c41c62be21efc0fa16b22b10e85b34
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482561
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Previously, the mini_ast used for flow analysis testing simulated
prefix increment/decrement operations using a `Write` node with a
null right-hand side. This commit introduces a dedicated `PreIncDec`
node to represent these operations more accurately.
The test "write() permits expression to be null" is removed in favor
of two new tests:
- "preIncDec() stores expressionInfo in the write"
- "preIncDec() demotes to the written type"
These new tests parallel the corresponding tests that already exist
for postIncDec.
Change-Id: I6a6a69646c63ca1c605272d4170eae3729ce90e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482560
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change makes two changes to how flow analysis works for post
increment/decrement operations.
Firstly, in the anaylzer implementation, the check against
`inference_update_4` is removed. Previously, the analyzer called
`FlowAnalysis.postIncDec` when `inference_update_4` was enabled, and
`FlowAnalysis.write` when it was disabled. This was unnecessary, since
that language feature had no effect on the flow analysis of post
increment/decrement. Furthermore, it was a violation of separation of
concerns, because even if that language feature _had_ had an effect on
the flow analysis of post increment/decrement, it would have been the
job of flow analysis to implement that effect, not the
client. Fortunately, `FlowAnalysis.write` happens to have the same
behavior as `FlowAnalysis.postIncDec` when `inference_update_4` is
disabled, so there is no behavioral change; this is purely a clean-up.
Secondly, the return type of `FlowAnalysis.postIncDec` is changed from
`ExpressionInfo?` to `void`, and the calls to `storeExpressionInfo`
are removed from call sites. Previously, `FlowAnalysis.postIncDec`
always returned `null`, so again, there is no behavioral change.
Note that these changes do not affect the front_end in any way, since
it de-sugars `x++` and `x--` to equivalent "let" expressions before
invoking flow analysis. Those let expressions, fortunately, have the
same flow analysis effect as `FlowAnalysis.postIncDec`. Thanks to the
test added in https://dart-review.googlesource.com/c/sdk/+/482342, we
have enough language test coverage to confirm this.
Change-Id: I6a6a6964253394c71e6bc72d801d9044b1b1ae30
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482541
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Adds a language test and a flow analysis unit test to cover a flow
analysis behavior of post-increment and post-decrement operators that
wasn't previously covered.
The tests verify that the expressions `x++` and `x--` demote `x` in
the same way that `x = x + 1` and `x = x - 1` would. This demotion is
only user-visible if the type of `x` is a user-defined type.
In the process of writing these tests, I noticed that the "mini-AST"
implementation of post-increment (which is used solely for flow
analysis unit testing) was not correct; it presumed that the type read
from the target, the type written to it, and the type of the whole
expression were all the same. This is not correct; the type written to
the target is determined by the return type of the `+` operator. I've
fixed this as part of this CL so that the unit test properly exercises
flow analysis.
I will follow this up with some refactoring of how flow analysis
handles post increment/decrement operations. Landing the test first
allows us to be confident that the refactor won't change the tested
behavior.
Change-Id: I6a6a6964417b48db0c1681c06d7418bd79e96357
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482342
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Removes the call to `storeExpressionInfo` from
`NullShortingMixin.finishNullShorting`, putting it instead in
overrides in the derived classes: `ResolverVisitor`,
`_MiniAstTypeAnalyzer`, and `InferenceVisitorImpl`.
With this change, there are no more calls to `getExpressionInfo` or
`storeExpressionInfo` in the shared type analysis logic; they are all
in clients.
This opens the door to changing how the clients track flow analysis
expression info.
Change-Id: I6a6a696480c8ce5f353f3c37ee2215f5bf9d606d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480802
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Removes calls to `getExpressionInfo` and `storeExpressionInfo` out of
`NullShortingMixin.startNullShorting`.
These calls are moved to the methods that call `startNullShorting`.
This reduces the reliance of `pkg/_fe_analyzer_shared` on
`getExpressionInfo` and `storeExpressionInfo`, which paves the way for
moving the exprssion info map to clients.
Change-Id: I6a6a69641d10db8acb0e1e45a26ca383ac9fe3ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480781
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Adds a `flowAnalysisInfo` field to the `ExpressionTypeAnalysisResult`
class. This field is populated with the `ExpressionInfo` that flow
analysis associates with the expression (or `null` if flow analysis
doesn't associate any `ExpressionInfo` with the expression).
The information in this field replicates the information stored in
`_FlowAnalysisImpl._expressionInfoMap`. Eventually all uses of
`_FlowAnalysisImpl._expressionInfoMap` will be transitioned to using
`ExpressionTypeAnalysisResult.flowAnalysisInfo` instead, and then
`_FlowAnalysisImpl._expressionInfoMap` will be removed.
Assertions are added to help ensure that the new field is populated
with sensible data:
- In the "mini-AST" representation used by `_fe_analyzer_shared` for
testing, an assertion checks that the value of
`ExpressionTypeAnalysisResult.flowAnalysisInfo` matches the value
stored in `_FlowAnalysisImpl._expressionInfoMap`.
- In the analyzer and front end, such an assertion wouldn't add any
value (since the front end and the analyzer make use of
`ExpressionTypeAnalysisResult` solely for interacting with shared
code), however assertions are added to verify the analyzer and front
end's conventions for handling expression infos for rewritten ASTs.
Change-Id: I6a6a696415fe48f5e24c33c50f1bccf55fff0f1f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480743
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Removes the method `FlowAnalysis.forwardExpression`, whose job was to
handle expressions that were rewritten during resolution, transferring
`ExpressionInfo` objects that were associated with the old expression
to the new rewritten expression.
Calls to `FlowAnalysis.forwardExpression` are replaced by an
equivalent construct: a call to `FlowAnalysis.getExpressionInfo` and a
call to `FlowAnalysis.setExpressionInfo`.
This change reduces the number of flow analysis methods that need to
interact with the map that associates expressions with
`ExpressionInfo` objects, paving the way for eventually removing that
map entirely.
Change-Id: I6a6a696486ea5bd30a6b2945f827a320fce1588b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480720
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the return type of `_MiniAstTypeAnalyzer.analyzeBoolLiteral`
to `ExpressionTypeAnalysisResult`. This makes it consistent with all
the other methods in `_MiniAstTypeAnalyzer` that operate on
expressions.
This paves the way for a follow-up CL that will expand
`ExpressionTypeAnalysisResult` so that it stores flow analysis
expression info.
Change-Id: I6a6a6964f146ad38e4cd044ead6fab23dedf21fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480761
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Changes the signature of `FlowAnalysis.whyNotPromoted` so that the
caller is responsible for looking up the expression info of the
matched value, and passing it in to flow analysis.
Change-Id: I6a6a69647bd1006c4ed4776a20bf36cd4c923498
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480620
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Changes the signature of `FlowAnalysis.parenthesizedExpression` so
that instead of the caller passing in the inner and outer expressions,
the caller is responsible for fetching the expression info of the
inner expression and storing the expression info for the outer
expression.
With this change, `FlowAnalysis.parenthesizedExpression` is
essentially a no-op (it just returns back the expression info it was
passed). So in principle we could remove it from the flow analysis API
entirely. But I think it's valuable to keep it around because it
clearly documents the behavioral intent (namely, the flow analysis
treats a parenthesized expression exactly the same way it treats the
inner expression).
Change-Id: I6a6a69647d41fb91017a3a42796418f61c009f09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480583
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is the second in a series of two CLs that changes the class
`ExpressionPropertyTarget` so that instead of looking up the target's
expression info during the call to `_getSsaNode`, it is given the
expression info at the time it is constructed. This will help simplify
the flow analysis API, by removing the need for it to know about the
`Expression` types used by the client.
In this second CL, the `expression` field is dropped from
`ExpressionPropertyTarget`. The `_getSsaNode` method no longer needs
to look up the expression info using `_getExpressionInfo`, since it is
supplied by the caller.
Change-Id: I6a6a69642c45a68d9eb580c1e7a28ea29bc07a65
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480581
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This is the first in a series of two CLs that will change the class
`ExpressionPropertyTarget` so that instead of looking up the target's
expression info during the call to `_getSsaNode`, it is given the
expression info at the time it is constructed. This will help simplify
the flow analysis API, by removing the need for it to know about the
`Expression` types used by the client.
In this first CL, an `expressionInfo` field is added to
`ExpressionPropertyTarget`, but the old `expression` field is not yet
removed. At the time `_getSsaNode` is called, it looks up the
expression info and double checks (using an assertion) that it's the
same as what is stored in its `expressionInfo` field.
Once this change has passed trybots, I'll follow up with the second CL
in the series, which will remove the `expression` field.
The reason I've split this into two CLs is that I've discovered that
at least one call site needed to have its logic reordered, to make
sure that it didn't try to call `getExpressionInfo` too early. Before
I proceed with the second CL, I want to see a full trybot run with the
assertion in place, to verify that the expression infos supplied to
`ExpressionPropertyTarget` are correct.
Change-Id: I6a6a69641bf8779743dda4e74ec6adbeac0ee2d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480600
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Changes the signatures of the following flow analysis methods:
- `FlowAnalysis.cascadeExpression_afterTarget`
- `FlowAnalysis.initialize`
- `FlowAnalysis.nullAwareMapEntry_valueBegin`
- `FlowAnalysis.write`
- `FlowAnalysisNullShortingInterface.nullAwareAccess_rightBegin`
so that the caller is responsible for looking up the expression info of
the matched value, and passing it in to flow analysis.
Additionally, the declaration of `getExpressionInfo` is moved from
`FlowAnalysis` to `FlowAnalysisNullShortingInterface`, in order to make
it available for calling in `NullShortingMixin`. With that change,
`getExpressionInfo` becomes declared in the same class as
`storeExpressionInfo`.
Change-Id: I2940663d00b46ee144fb1f86273ed87ae78a9fff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480160
Reviewed-by: Paul Berry <paulberry@google.com>
Auto-Submit: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the signatures of the following flow analysis methods:
- `ifNullExpression_rightBegin`
- `nonNullAssert_end`
so that the caller is responsible for looking up the expression info
of the matched value, and passing it in to flow analysis.
Change-Id: I6998042a3048c0d9c2cc5967d1f6f3e0ce744435
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/479842
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Auto-Submit: Chloe Stefantsova <cstefantsova@google.com>
Changes the signatures of the flow analysis method
`equalityOperation_end` so that the caller is responsible for looking
up the expression info of the matched value, and passing it in to flow
analysis.
Change-Id: Ic12274a21b4197018ac7ba2057b49589c0737f53
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/478960
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Auto-Submit: Chloe Stefantsova <cstefantsova@google.com>
Changes the signatures of the following flow analysis methods:
- `asExpression_end`
- `isExpression_end`
so that the caller is responsible for looking up the expression info
of the matched value, and passing it in to flow analysis.
Change-Id: Ifd51c2e0feb7b02f8bec2f08e74138064828a4ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/478601
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Changes the signature of `FlowAnalysis.propertyGet`, so that in
addition to returning the promoted type of the variable, it returns an
`ExpressionInfo`, which the caller is responsible for storing.
Change-Id: I6a6a6964b7d3a1f8509433383ab7e70afabe99f1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474921
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the signature of the following flow analysis methods:
- `cascadeExpression_end`
- `nullAwareAccess_rightBegin`
so that they return an `ExpressionInfo`, which the caller is
responsible for storing.
Also changes `cascadeExpression_afterTarget` so that it no longer
passes the return value from `_nullAwareAccess_rightBegin` to
`_storeExpressionInfo`. This is unnecessary because the target of a
cascade is never be used as an argument to `_getExpressionInfo`.
Finally, changes the call sites of `nullAwareAccess_end` that do not
have to do with cascades, so that they are responsible for clearing
out the full expression info (by calling
`storeExpressionInfo(wholeExpression, null)`. This was previously done
by `nullAwareAccess_end` itself. (This is not necessary for cascades
because in cascades, the call to `nullAwareAccess_end` is always
followed by a call to `cascadeExpression_end`, and the caller stores
the expression info after *that* call).
Note that since one of the callers to `storeExpressionInfo` is now in
`NullShortingMixin` (which only has access to flow analysis via the
abstract base class `FlowAnalysisNullShortingInterface`), the abstract
declaration of `storeExpressionInfo` had to be moved from
`FlowAnalysis` to `FlowAnalysisNullShortingInterface`.
Change-Id: I6a6a696472bb914d408346b1ec8c264f6bc5f63f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475160
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Changes the signatures of the following flow analysis methods:
- `assert_afterCondition`
- `conditional_elseBegin`
- `conditional_end`
- `conditional_thenBegin`
- `doStatement_end`
- `ifStatement_thenBegin`
- `logicalBinaryOp_end`
- `logicalBinaryOp_rightBegin`
- `whileStatement_bodyBegin`
so that the caller is responsible for looking up the expression info
of the matched value, and passing it in to flow analysis.
Change-Id: I6a6a6964c2f975a9baf16bca5b92693af5d9fb41
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475126
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the signature of the flow analysis method `for_bodyBegin` so
that the caller is responsible for looking up the expression info for
the loop condition. Since expression info is sometimes `null`, it's no
longer possible to use `null` to signify the absence of a loop
condition. This is now done by passing the value returned by a call to
`booleanLiteral(true)`. This has the advantage of guaranteeing, by
construction, that the flow analysis of an empty loop condition is the
same as the flow analysis of a loop condition of `true`.
Change-Id: I6a6a696436b7d7c3b8d3171360518dc05d42e3d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475163
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Changes the signature of the following flow analysis methods:
- `conditional_end`
- `equalityOperation_end`
- `isExpression_end`
- `logicalBinaryOp_end`
so that in addition to returning the promoted type of the variable, it
returns an `ExpressionInfo`, which the caller is responsible for
storing.
Change-Id: I6a6a6964d5f53fb7eafa04d1ad5dd83b28dbf527
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474924
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the signature of `FlowAnalysis.variableRead`, so that in
addition to returning the promoted type of the variable, it returns an
`ExpressionInfo`, which the caller is responsible for storing.
Change-Id: I6a6a69640d59a164aaf6621a9c9ee5038ed16864
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474923
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Changes the signature of the following flow analysis methods:
- `booleanLiteral`
- `nullLiteral`
- `thisOrSuper`
so that in addition to returning the promoted type of the variable, it
returns an `ExpressionInfo`, which the caller is responsible for
storing.
Change-Id: I6a6a6964bd5fe4d620d702deb2313f07393b5a0e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474922
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the signature of the following flow analysis methods, which
all perform local variable writes, so that they return an
`ExpressionInfo`, which the caller is responsible for storing.
- `postIncDec`
- `write`
- `_write`
Note that `assignedVariablePattern` also performs a local variable
write, but since it operates on a pattern rather than expression, no
expression info needs to be stored, so it still returns `void`.
Change-Id: I6a6a69640fa889db1fa58b6614ec855e365a8798
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/474860
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Changes the API of `FlowAnalysis.logicalNot_end` so that it accepts an
`ExpressionInfo` object describing the subexpression, and returns an
`ExpressionInfo` object describing the whole expression; the client is
responsible for getting the `ExpressionInfo` object prior to the call
and storing it afterwards.
In the process of making this change, I noticed the opportunity to
make a small optimization: when the subexpression of a logical not
expression has no associated `ExpressionInfo`, it is not necessary to
create an `ExpressionInfo` for the whole logical not expression.
Eventually all flow analysis APIs will be adjusted in a similar way,
so that the clients become fully responsible for keeping track of the
association between expressions and `ExpressionInfo` objects. Once
that is done, clients will be free to maintain the mapping between
expressions and `ExpressionInfo` objects in whatever way they see fit,
they won't be required to store the assocation in a map.
Change-Id: I6a6a6964313da85a94a52c5b59b663568058d84d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471981
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
The return types of the following methods are changed to
`ExpressionTypeAnalysisResult`:
- `NullShortingMixin.finishNullShorting` (previously returned
`SharedTypeView`).
- `NullShortingMixin.handleNullShortingStep` (previously returned
`void`).
- `TypeAnalysisNullShortingInterface.finishNullShorting` (previously
returned `SharedTypeView`).
- `TypeAnalyzer.analyzeExpression` (previously returned
`SharedTypeView`).
With one exception, these methods previously returned a
`SharedTypeView` representing the expression's static type. (The
exception was `NullShortingMixin.handleNullShortingStep`, which
previously returned `void`).
This paves the way for allowing these methods to return additional
information beyond the static type of the expression, such as flow
analysis results and tree rewrite information. (This information is
currently carried around in auxiliary data structures, which leads to
bookkeeping headaches and increases the risk of bugs.)
Change-Id: I6a6a6964572a5fd7fac5496cdbfbc82a10272bba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471323
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Changes the behavior of `FlowAnalysis.cascadeExpression_afterTarget`
so that if the cascade is null-aware, flow analysis automatically
accounts for the resulting control flow split. Previously, the caller
had to account for the control flow split via a separate call to
`FlowAnalysis.nullAwareAccess_rightBegin`.
This is accomplished by extracting the implementation of
`FlowAnalysis.nullAwareAccess_rightBegin` to a private internal
method, and calling it from
`FlowAnalysis.cascadeExpression_afterTarget`.
This paves the way for a chain of follow-up CLs that will simplify how
flow analysis associates `ExpressionInfo` objects with expressions. It
makes the follow-up CL chain simpler by avoiding the need for some
complex gymnastics to carry an `ExpressionInfo` object between the
call to `FlowAnalysis.cascadeExpression_afterTarget` and a follow-up
call to `FlowAnalysis.nullAwareAccess_rightBegin`.
Note that this change creates some asymmetry, because it is still
necessary for the caller to call `FlowAnalysis.nullAwareAccess_end`
before calling `cascadeExpression_end`. I will address this in a
follow-up CL; for now I've left a TODO comment in
`_FlowAnalysisImpl.cascadeExpression_end`.
Change-Id: I6a6a696483dec4eb4770f63a05cd7c0e16443ae4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471341
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The code in question already handles null shorting, thanks to the
shared implementation of null shorting in `NullShortingMixin`, so the
TODO "handle null shorting" is no longer needed.
Change-Id: I6a6a6964a84c3dea495a52dcdb16a21554259eae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471340
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Removes the type argument `Type` from the classes `MatchContext` and
`TypeAnalyzerErrors`.
There is no change in functionality, since all these type arguments
were always being instantiated with the same type argument
(`SharedTypeView`).
Change-Id: I6a6a6964b17aa22c145975e457cbf301842e69a5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469921
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Removes the type argument `Type` from the `FlowAnalysis` class and
related classes.
There is no change in functionality, since all these type arguments
were always being instantiated with the same type argument
(`SharedTypeView`).
Change-Id: I6a6a6964698494c8df721ae422080032aaaf7dcf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469920
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Changes the test types used in the flow analysis `joinPromotionChains`
tests from type `Type` to type `SharedTypeView`. There is no change in
functionality.
This paves the way for a follow-up CL that will change all the flow
analysis logic to use the `SharedTypeView` extension type.
Change-Id: I6a6a6964eadaa414b60537125427b5205b49772f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469900
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This reverts commit c616db31d2.
Reason for revert: Fix landed downstream in Flutter engine: https://github.com/flutter/flutter/pull/180127
Original change's description:
> Revert "[ DDC / CFE ] Add support for allowing imports of unsupported libraries"
>
> This reverts commit b5e60be49d.
>
> Reason for revert: broke Flutter web engine tests
>
> Original change's description:
> > [ DDC / CFE ] Add support for allowing imports of unsupported libraries
> >
> > This change adds support for allowing for imports of unsupported
> > platform-specific libraries when the
> > `--include-unsupported-platform-library-stubs` flag is provided to the
> > CFE.
> >
> > This flag sets the `includeUnsupportedPlatformLibraryStubs` property in
> > `TargetFlags`, which `Target`s can use to conditionally return different
> > `DartLibrarySupport` objects with different supported/unsupported
> > library sets.
> >
> > A `checkForUnsupportedDartColonImports` function has been added to
> > `Target` that uses the value of `dartLibrarySupport` to determine if
> > there's any unsupported library imports. This function is called after
> > the various transformation operations provided by the `Target`
> > implementation, meaning the import of an unsupported library specified
> > in `dartLibrarySupport` will now result in a compilation error (this
> > includes `dart:mirrors` imports for VM targets when mirrors are
> > disabled, which was previously handled by the VM itself).
> >
> > Related to https://github.com/dart-lang/sdk/issues/62125
> >
> > TEST=Tests added / modified
> >
> > Change-Id: Ife819b2e1a6d28f67d80aab6701cd23a1724aa4d
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465760
> > Reviewed-by: Nicholas Shahan <nshahan@google.com>
> > Reviewed-by: Johnni Winther <johnniwinther@google.com>
> > Commit-Queue: Ben Konyi <bkonyi@google.com>
>
> Change-Id: I0b59f00e55a2424f783351abd977eb38409ce01f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469100
> Reviewed-by: Nate Biggs <natebiggs@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
Change-Id: I1ae2eac675432286aebabea3c1f58caf35a27fbb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469240
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This reverts commit b5e60be49d.
Reason for revert: broke Flutter web engine tests
Original change's description:
> [ DDC / CFE ] Add support for allowing imports of unsupported libraries
>
> This change adds support for allowing for imports of unsupported
> platform-specific libraries when the
> `--include-unsupported-platform-library-stubs` flag is provided to the
> CFE.
>
> This flag sets the `includeUnsupportedPlatformLibraryStubs` property in
> `TargetFlags`, which `Target`s can use to conditionally return different
> `DartLibrarySupport` objects with different supported/unsupported
> library sets.
>
> A `checkForUnsupportedDartColonImports` function has been added to
> `Target` that uses the value of `dartLibrarySupport` to determine if
> there's any unsupported library imports. This function is called after
> the various transformation operations provided by the `Target`
> implementation, meaning the import of an unsupported library specified
> in `dartLibrarySupport` will now result in a compilation error (this
> includes `dart:mirrors` imports for VM targets when mirrors are
> disabled, which was previously handled by the VM itself).
>
> Related to https://github.com/dart-lang/sdk/issues/62125
>
> TEST=Tests added / modified
>
> Change-Id: Ife819b2e1a6d28f67d80aab6701cd23a1724aa4d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465760
> Reviewed-by: Nicholas Shahan <nshahan@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>
Change-Id: I0b59f00e55a2424f783351abd977eb38409ce01f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469100
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Erik Ernst