Previously, `attachFinally` took advantage of
`Reachability.rebaseForward` to compute reachability after a
`try/finally` statement. This was overkill, since
`Reachability.rebaseForward` contains special logic to account for
widely diverging reachability stacks, whereas `attachFinally` always
deals with reachability stacks that have an immediate common parent.
This change adds an assertion to verify that the reachability stacks
always have an immediate common ancestor, and replaces the call to
`Reachability.rebaseForward` with the equivalent direct logic.
Making this change will simplify the process of writing a spec for
flow analysis.
Change-Id: Icd7d3ebc150d1836932efe3891b3ea1077134351
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437300
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The reports the declaration conflicts through the _Declaration objects,
which streamlines the code and helps providing more targeted messages.
Change-Id: I013c9974de87c5a1c541ba6aa5d78a1ba63e1074
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437241
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
While doing specification work for flow analysis, I noticed that in
circumstances where a flow model needs to have both the `split` and
`conservativeJoin` operations applied to it, sometimes flow analysis
applied `split` first, and sometimes it applied `conservativeJoin`
first.
The actual order doesn't matter, since `split` only affects the flow
model's reachability, and `conservativeJoin` only affects the flow
model's variable info. But for the purpose of making the specification
clearer, it's nice to be consistent. This change adjusts the order so
that `split` always happens before `conservativeJoin`.
Change-Id: Ia5dfc9335ee85f122ae274f6bb850f83f12b88d2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437060
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change updates the mini_ast testing infrastructure so that:
- It properly models the "exception type" part of a catch clause (the
type named after the `on` keyword).
- It requires an exception variable to be specified if there is a
stack trace variable (this is required by the Dart grammar).
- It requires an exception type to be specified if there is no
exception variable (this is required by the Dart grammar).
- During the "pre-visit" stage, the exception variable and stack trace
variable are registered with the `AssignedVariables` object, so that
they can be properly handled by type promotion.
- During the main "visit" stage, the exception variable and stack
trace variable are assigned the appropriate types.
Flow analysis unit tests are updated in order to meet the new
requirements, and flow analysis tests are added to check that stack
trace and exception variables are promotable and appropriately typed.
By ensuring that stack trace and exception variables are properly
typed during flow analysis tests, this paves the way for some
follow-up work, in which I plan to re-work how flow analysis keeps
track of variable types.
There is no behavioral change to the analyzer or compiler pipeline;
these changes are confined to `pkg/_fe_analyzer_shared/test`.
Change-Id: I49c4b894d82d1dc58d62e3d3f25d232c9106922e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/434145
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Inspired by some recent code reviews from Lasse, I decided to make
some improvements to flow analysis documentation to bring it more in
line with the Dart style guide.
Change-Id: Ibbda802055d7e17429806b2a4e608005bf761a69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/433380
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change introduces some helper functions to avoid duplication
between the enabled/disabled variants of each of the try/finally
ordering tests.
Thanks to Lasse for the suggestion.
Change-Id: I4538d44643fde7954a75bc1b6843fcc805d8afc4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432782
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This change adds test-only methods `propertyPromotionChainForTesting`
and `variablePromotionChainForTesting` to flow analysis; these are
used by the flow analysis unit tests to query the full promotion chain
of a porperty (or variable, respectively). This allows tests to
observe the effect of try/finally ordering on promotion chains without
having to resort to clever control flow joins.
Thanks to Lasse for the suggestion.
Change-Id: Ida5349ecc93dbd4b3392becd20882be3eb101024
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432800
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
When an `is` test is trivially satisfied (i.e. `expr is T`, when the
static type of `expr` is a subtype of `T`), the `is` test is
guaranteed by soundness to evaluate to `true`, so any code path that
follows from the `is` test evaluating to `false` is unreachable.
This reasoning wasn't valid prior to sound null safety, because in
mixed mode programs, it was possible for an expression to evaluate to
`null` even if its static type wasn't nullable, and hence `expr is T`
might evaluate to `false` even if the static type of `expr` was a
subtype of `T`. So this change is gated on the `sound-flow-analysis`
language flag (which is enabled in Dart 3.9).
Fixes https://github.com/dart-lang/sdk/issues/60718.
Change-Id: I66a65580b738162f23b6fb468b71fcac66bfbb95
Bug: https://github.com/dart-lang/sdk/issues/60718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431740
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
During the implementation of flow analysis for patterns, I added a
hack to the flow analysis tests so that all variable declarations were
desugared into the equivalent pattern variable declaration. This had
the advantage of getting some extra testing "for free" during early
development of the patterns feature, but it had a few disadvantages:
- It the caused the shared flow analysis and type inference tests to
stop exercising flow analysis code paths for ordinary variable
declarations.
- It forced the testing logic to "support" pattern variable
declarations that are late or lack an initializer; these are things
that regular Dart doesn't support, and they made the implementation
of `PatternVariableDeclaration.visit` much more complicated than it
needed to be.
The language team is currently contemplating some changes to how
promotions work in ordinary variable declarations and pattern variable
declarations (see, for example,
https://github.com/dart-lang/language/issues/4347#issuecomment-2861859329). So
in order to be able to experiment with these possibilities, I want to
get rid of this hack.
This CL adds a `VariableDeclaration` class, to represent ordinary
variable declarations, and modifies the `declare` function (which is
used by tests for creating an ordinary variable declaration) so that
it creates a `VariableDeclaration`, rather than rewriting it into a
`PatternVariableDeclaration` containing a `VariablePattern`. It
removes "support" for testing pattern variable declarations that are
late or lack an initializer, simplifying
`PatternVariableDeclaration.visit`.
A few test cases in `type_inference_test.dart` were checking the
generated IR for ordinary variable declarations, verifying that it was
properly desugared into a pattern variable declaration; these
expectations have accordingly changed to no longer expect desugaring.
There is no functional change to the compiler or analyzer
toolchain. These changes only affect tests in
`pkg/_fe_analyzer_shared/test`.
Change-Id: I90f074cd6e7a6ed5ed11afadeadb6131ce1d282f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432122
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
The following renames are performed:
- The class `Declare` (which is the "mini-AST" representation of a
pattern variable declaration), is renamed to
`PatternVariableDeclaration`.
- The top level function `match` (which is used in tests to construct
a "mini-AST" representation of a pattern variable declaration), is
renamed to `patternVariableDeclaration`.
The new names should help avoid confusion in a follow-up CL I intend
to create, which will introduce a new `VariableDeclaration` class to
represent ordinary variable declarations.
There is no functional change. These renames only affect tests in
`pkg/_fe_analyzer_shared`.
Change-Id: Ic6b6f75cbb312180273a08f2c3926f83da254bc0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432120
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, when assiging to a local variable that was promoted, if
the newly assigned value was not compatible with any of the promotions
(i.e., the variable was fully demoted back to its declared type), then
the set of types of interest was cleared.
This behavior was not documented anywhere in the spec, and it seems
oddly inconsistent to me; as far as I can tell, flow analysis doesn't
clear types of interest in any other circumstances. I've looked
through git history as well as my personal notes, and I've been unable
to find any justification for this behavior. So, with the agreement of
the language team, I'm removing it when sound-flow-analysis is
enabled.
Fixes https://github.com/dart-lang/language/issues/4380.
Bug: https://github.com/dart-lang/language/issues/4380
Change-Id: Ic1ca80a61e21482e659afa8796b08fce707db3c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429227
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This gets rid of (what is described in commented code as)
InstantiatedTypeWithArgumentsTest, Subtype1TestCacheLookup, BoolTest.
Running this through the benchmarker like this:
```
out/ReleaseX64/dart-sdk/bin/dart pkg/front_end/tool/benchmarker.dart \
--snapshot=pkg/analysis_server/bin/server.aot.1 \
--snapshot=pkg/analysis_server/bin/server.aot.2 \
--arguments="--disable-file-byte-store" \
--arguments="--train-using" \
--arguments="pkg/front_end/lib/"
```
(there `server.aot.1` is without this CL and `server.aot.2` is with)
I get:
```
msec task-clock:u: -0.9614% +/- 0.3557% (-78.23 +/- 28.95) (8136.80 -> 8058.57)
cycles:u: -1.0257% +/- 0.3626% (-348283560.66 +/- 123129577.69) (33955507948.94 -> 33607224388.28)
instructions:u: -1.0595% +/- 0.0662% (-479117043.74 +/- 29935955.93) (45220681554.68 -> 44741564510.94)
seconds time elapsed: -0.9548% +/- 0.3540% (-0.08 +/- 0.03) (8.15 -> 8.07)
seconds user: -1.0649% +/- 0.4031% (-0.08 +/- 0.03) (7.80 -> 7.72)
Scavenge( new space) goes from 130 to 129
Notice combined GC time goes from 2613 ms to 2608 ms (notice only 1 run each).
```
`StackImpl.pop` as reported by `perf report --no-children` goes from
0.81% to 0.34% (and I'm unsure why the apparent change seen as a whole
is quite a bit bigger).
Change-Id: Ife36da11215e0c8aa706d945f0ee5018ad03ab91
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432340
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
A tricky part of the implementation of flow analysis is the handling
of try/finally statements. Although promotions are tracked separately
in the `try` and `finally` blocks, promotions from both blocks need to
be merged together at the conclusion of the finally block. This
creates an ambiguity, because each type in a promotion chain is
required to be a subtype of the previous, and hence multiple
promotions of the same variable are inherently ordered. The ambiguity
is: when the promotions from the `try` and `finally` block are merged,
which promotions should be applied first?
In discussion with the language team, we've decided that the
promotions from the `try` block should be applied first, because that
matches the order of code execution. This change makes the behavior of
flow analysis more uniform, which should make it easier to reason
about and maintain.
In practice, the difference in behavior is quite subtle, and I don't
expect users to notice. However, to be on the safe side, the change in
behavior is conditioned on the `sound-flow-analysis` flag, so it will
only take effect when the user deliberately upgrades to language
version 3.9, and it will not affect already-published packages.
A test in google3 showed that no internal code would be broken by
force-enabling this change.
Fixes https://github.com/dart-lang/language/issues/4382.
Change-Id: I0e9f6db808a964e0b4325d3020654a9f2be273a2
Bug: https://github.com/dart-lang/language/issues/4382
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432001
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, flow analysis had the rule that type promotion only
occurred when the type being tested was a subtype of the previously
promoted type (or the declared type, if there was no previous
promotion). This led to counterintuitive behaviors when the type being
tested and the previously promoted type were mutual subtypes (see
https://github.com/dart-lang/language/issues/4368).
With this change, the rule is updated so that type promotion only
occurs when the type being tested is a subtype of the previously
promoted type _and_ the previously promoted type is _not_ a subtype of
the type being tested. The user-visible difference is that promotion
to a mutual subtype no longer occurs.
This change makes flow analysis easier to reason about, and improves
its behavior in corner cases, but I believe it will have minimal
impact on real-world code. But to reduce the risk to existing code,
the change only takes effect when the `sound-flow-analysis` language
feature is enabled.
Fixes https://github.com/dart-lang/language/issues/4368.
Bug: https://github.com/dart-lang/language/issues/4368
Change-Id: I30dab017e043e75603d618df721c8a2683667cd5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429200
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I accidentally called the test group "Sound null safety", which is a
related concept, but not the same thing.
Sound null safety is a compilation mode in which no legacy code is
allowed, and so it is sound to assume that an expression with a
non-nullable type cannot evaluate to `null`. It has been the only
allowed way to compile Dart programs for some time.
Sound flow analysis, on the other hand, is a set of improvements to
flow analysis which are possible now that all Dart programs are now
compiled in sound null safety mode. It is a language-versioned feature
that is enabled in Dart 3.9.
Change-Id: Ib13d28be7fecea6d0e931b33d6643afab2349629
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432004
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Several call sites intentionally discard a future; these call sites
have been wrapped with `unawaited()` (which is the standard way to
suppress the lint).
Turning on this lint uncovered several tests that should have been
async; these tests have been fixed.
Change-Id: I66e0f6bf7a05946e8d460ce9b5581837ce067870
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431420
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Default types are used in the exhaustiveness computation but were not
computed for local functions, leading to a crash during constant
evaluation. This adds computation of default types to
FunctionDeclaration and FunctionExpression.
Closes#54388Closes#60264
Change-Id: I95eb2ae5b42e4fb5ab77088c915fe6088a1d4060
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427702
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This PR enables native assets on the main and dev channel by default,
and make native assets available on the beta channel.
This PR removes the flag from invocations.
The helper packages (`package:hooks` and `package:code_assets`) will
stay 0.x for now, until the SDK constraint can be bumped to a beta
release and we're happy with the Dart API.
`dart build` is also made available as preview (without a flag on
the main, dev, and beta channels). We're still finalizing the spec for
this command. (https://github.com/dart-lang/sdk/issues/60730)
`dart test` will need https://github.com/dart-lang/test/pull/2501.
This means users will need to update their `package:test` dependency.
This PR refactors the way that invalid `package_config.json`s are
handled: they are now loaded in the dartdev commands and handled there.
Bug: https://github.com/dart-lang/sdk/issues/50565
Project: https://github.com/orgs/dart-lang/projects/99/
Change-Id: I7db9ff6d7196750cab9379a4605c6bbf89a974d7
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-win-release-arm64-try,pkg-mac-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429920
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The way this is accomplished is that in
`_FlowAnalysisImpl.nullAwareAccess_rightBegin`, any expression
reference associated with the target of the null-aware access is
restored, and the corresponding SSA node is associated with the guard
variable (if any). These changes ensure that if the null-aware access
is a property get, the subsequent call to `propertyGet` will pick up
the appropriate SSA node, so it will be able to locate the promotion
key for the property.
This functionality is only enabled when the language feature
`sound-flow-analysis` is enabled.
To prevent test regressions, a few related changes need to be made at
the same time:
- `_FlowAnalysisImpl.nullAwareAccess_end` is changed so that it clears
any expression info or expression reference that was associated with
the null-aware access expression. This prevents flow analysis
information from being erroneously propagated out of a null-aware
expression, which would have led to assertion failures when
analyzing null-aware expressions inside of conditional
expressions. This wasn't previously a problem because the expression
reference used to be consumed by
`_FlowAnalysisImpl.nullAwareAccess_rightBegin`, preventing further
expression references and expression infos from being recorded
further along in the null-aware access.
- The test framework in `mini_ast.dart` is fixed so that `!` is
considered to participate in null shorting. This was a bug in the
test framework that wasn't previously caught because it happened not
to produce any test failures.
- The analyzer's method `PostfixExpressionResolver._resolveNullCheck`
is changed so that it calls `nonNullAssert_end` before terminating
null-aware access. Previously, the order was swapped, causing
`nullAwareAccess_end` to be called before `nonNullAssert_end` when
analyzing expressions like `a?.b!`. This used to be benign, but now
that non-cascaded field accesses participate in field promotion,
flow analysis needs the methods to be called in the correct order.
Fixes https://github.com/dart-lang/language/issues/4344.
Bug: https://github.com/dart-lang/language/issues/4344
Change-Id: I523be1b4be1af3f68654a745187a546728c878fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427820
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
The CFE desugars null-aware expressions such as `a?.b` into equivalent
`let` expressions involving a temporary guard variable (e.g., `let x =
a in x == null ? null : x.b`). This desugaring happens in the body
builder, prior to flow analysis. As a result, the expressions and
variables passed to flow analysis are those of the desugared `let`
expression rather than those of the code the user wrote. So a hack is
needed to ensure that flow analysis produces the correct result; the
hack involves promoting the temporary guard variable in the code path
where it is not null.
Prior to this change, the hack was done entirely in the CFE. Instead
of making a single pair of calls to
`FlowAnalysis.nullAwareAccess_rightBegin` and
`FlowAnalysis.nullAwareAccess_end` for every null-aware expression,
the CFE made two nested pairs of calls: one to promote the true target
of the null-aware access, and the other to promote the temporary guard
variable.
This had the advantage of keeping the hack entirely in the CFE, but
unfortunately it got in the way of fixing
https://github.com/dart-lang/language/issues/4344, because it
prevented flow analysis from properly recognizing field accesses
inside null-aware expressions.
This change adds an optional `guardVariable` parameter to
`FlowAnalysis_nullAwareAccess_rightBegin`, and shifts the
responsibility for promoting the temporary guard variable to the flow
analysis engine itself. With this change, the CFE no longer needs to
make two nested pairs of calls to
`FlowAnalysis.nullAwareAccess_rightBegin` and
`FlowAnalysis.nullAwareAccess_end`, and flow analysis now has the
necessary information to recognize field accesses inside null-aware
expressions.
I will perform the actual fix for
https://github.com/dart-lang/language/issues/4344 in a follow-up CL.
Note that in the future, I would like to change the CFE so that it
desugars null-aware accesses during type inference rather than during
the body builder; this will allow the expressions and variables passed
to flow analysis to be precisely those of the code the user wrote;
that in turn will give us extra confidence that flow analysis produces
the same results in the analyzer and the CFE.
Change-Id: Ie63619a3ca0f011c1ae8e3c0c76b5bc7c1ab8419
Bug: https://github.com/dart-lang/language/issues/4344
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427341
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This adds reporting of an error in CFE for ?.length in constant expressions and improves the message for the analyzer in the same case. The error in the analyzer was previously the invalid claim that
The property 'length' can't be accessed on the type 'Null' in a constant expression.
Closes#60509
Change-Id: Ibbe0fa1ace3bea9d83efea2ccf3ea9716a125d74
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421841
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This adds a `debugPrint` method along with methods to support automatic indentation.
The debug helpers are reexported inside package:analyzer and package:front_end for easy access.
Change-Id: I80a4103e6fb30a6b459551184916c3fa26591a20
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425280
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Context:
A while back, I inserted a synthetic token in the parser to allow us to parse without crashing the analyzer while I worked on the CFE. Now that both implementations are up and working, we can remove this synthetic token and allow the parser to parse as if we had enabled dot shorthands by default, and produce an error if the experiment isn't enabled.
This change allows a bunch of different language tests to start passing since they were blocked on the weird synthetic token messing up the parsing stream.
Bug: https://github.com/dart-lang/sdk/issues/59758
Change-Id: I792e3b917a76241b04ee708de06f670bbde64036
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423563
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Instead of validating the erasure of an extension type,
validate that extension type declaration and type arguments
are specified as callable.
TEST=pkg/dynamic_modules/test/data/extension_type2
Fixes b/411433443
Change-Id: I063f0a622abd0d75594c4095573b29a7888f5afb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423882
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
When there's exactly a dot shorthand of the RHS of `==`, we use the namespace of the LHS to resolve the shorthand on the RHS.
There's existing CFE work done for the `case ==` case in the type analyzer, so we just have to make sure `isDotShorthand` is properly set. This will take care of the relational pattern `==` behaviour.
Tested using language tests and resolution unit tests.
Bug: https://github.com/dart-lang/sdk/issues/59835
Change-Id: I3823641d1872bca68e344bc323209b58fb5098db
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422941
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Paul Berry