Commit Graph

321 Commits

Author SHA1 Message Date
Paul Berry 4ab18dac68 Shared type analysis for patterns: rename finishStatementCase
The new name, `handleMergedStatementCase`, is more consistent with the
rest of the type analyzer's `handle` methods, and more accurately
describes when the method is called (after a switch body that's
potentially shared by multiple case heads and possibly a `default`
clause).

Change-Id: I4f3166d5f58432f9f1cc0edffb3c0a317539ea23
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260064
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-20 20:37:09 +00:00
Paul Berry 380a505b0d Shared type analysis: add more pattern types.
Support for the following pattern types is added to the (as yet
unused) shared type analysis prototype:

- Cast patterns
- List patterns
- Logical-and patterns
- Logical-or patterns
- Null-assert patterns
- Null-check patterns
- Wildcard patterns

Change-Id: I923df94b5deef925ca94e6ff0c8eac0493f69c1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257602
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-09-20 17:15:15 +00:00
Paul Berry 84b71e55d4 Shared type analysis for patterns: clean up switch data structures.
- `CaseHeadInfo` is renamed to `CaseHeadOrDefaultInfo`, to reflect the
  fact that it is used for both case heads and default clauses.

- `CaseHeadOrDefaultInfo.node` is no longer needed; this used to be
  used for error reporting, but after the refactor of
  https://dart-review.googlesource.com/c/sdk/+/259021 is was no longer
  used.

- `ExpressionCaseInfo` is renamed to `SwitchExpressionMemberInfo`,
  consistent with the AST structure `SwitchExpressionMember` in the
  analyzer.

- `SwitchExpressionMemberInfo.body` is renamed to
  `SwitchExpressionMemberInfo.expression`, consistent with the
  nomenclature used in the analyzer.

- `StatementCaseInfo` is renamed to `SwitchStatementMemberInfo` for
  consistency with `SwitchExpressionMemberInfo`.  Note that the
  analyzer calls its corresponding AST structure `SwitchMember` rather
  than `SwitchStatementMember` for legacy reasons.

- `SwitchExpressionMemberInfo` no longer extends
  `CaseHeadOrDefaultInfo`; it contains a pointer to the head or
  default info.  This is more consistent with
  `SwitchStatementMemberInfo`.

Change-Id: I727766a6f0601ec5cd8aff824364319a54446bd2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259880
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-20 14:03:35 +00:00
Paul Berry 50ac31f286 Shared pattern logic: clean up nomenclature around guards.
Change-Id: I596362dab53ac2efc68cb45dbb29feab80c6240a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259463
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-20 13:02:16 +00:00
Chloe Stefantsova f8ef60a868 Revert "Reland "[cfe,corelib] Add class 'Record' to the core library""
This reverts commit 4b6a8f35b9.

Reason for revert: Breakages in google3.

Original change's description:
> Reland "[cfe,corelib] Add class 'Record' to the core library"
>
> Part of https://github.com/dart-lang/sdk/issues/49713
>
> Change-Id: I56bfca49492d14bb561b32993fd9adfe775b7400
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259583
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Slava Egorov <vegorov@google.com>
> Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I61c4db244329615d7d218484cd86601b1c737ba6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259800
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Auto-Submit: Chloe Stefantsova <cstefantsova@google.com>
2022-09-19 13:52:01 +00:00
Ahmed Ashour 5fe480b788 Fix typos
Fix #49864

TEST=ci

Change-Id: I9a7e06d604cd0b4f56f2ac229ab3fc9f01cb9d76
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256824
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
2022-09-16 19:35:00 +00:00
Paul Berry d695d95263 Shared type analysis: add "if-case" support.
Change-Id: I7c956239dd050c3c07ff85508180c451a70ca8a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257601
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-16 13:41:31 +00:00
Chloe Stefantsova 4b6a8f35b9 Reland "[cfe,corelib] Add class 'Record' to the core library"
Part of https://github.com/dart-lang/sdk/issues/49713

Change-Id: I56bfca49492d14bb561b32993fd9adfe775b7400
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259583
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
2022-09-16 11:46:40 +00:00
Alexander Aprelev d31c741fbb Revert "[cfe,corelib] Add class 'Record' to the core library"
This reverts commit f553f0aad9

Reason for revert: breaks g3 bot

Original change's description:
> [cfe,corelib] Add class 'Record' to the core library
>
> Part of https://github.com/dart-lang/sdk/issues/49713
>
> Change-Id: Ibb0309f97565c0a623f60e588eefcad6759d5ace
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257066
> Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

TBR=lrn@google.com,scheglov@google.com,cstefantsova@google.com,johnniwinther@google.com

Change-Id: If386819fc7419a660c692e4187ead4ebf142c68c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259462
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
2022-09-15 17:45:50 +00:00
Paul Berry a74d3fc01d Shared type analysis for patterns: update to latest spec.
Make the following changes, based on
https://github.com/dart-lang/language/commit/8a9b9a8a74a49e13c625e5208bab201d8a4f0e76:

- Replace `ConstOrLiteralPattern` nomenclature with `ConstantPattern`
  (the spec no longer speaks of "literal patterns").

- It is an error if a guard's type is not assignable to `bool`.

- Variable patterns can now be `final`.

- We now have a separate error condition to cover the case where a
  variable, list, map, record, or extractor pattern appears in an
  irrefutable context and the matched type is not assignable to the
  required type of the pattern.  (Previously such patterns were simply
  called "refutable", leading to a less clear error).

Additionally, we now consistenly use the term "guard" to refer to the
expression after a `when`, consistent with the spec text.

There are a few new TODOs, which I plan to address in follow-up CLs.

Change-Id: Ia0abab9492583f2aa8b59a9b381b90ba11b3e0fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259246
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-15 15:26:43 +00:00
Chloe Stefantsova f553f0aad9 [cfe,corelib] Add class 'Record' to the core library
Part of https://github.com/dart-lang/sdk/issues/49713

Change-Id: Ibb0309f97565c0a623f60e588eefcad6759d5ace
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257066
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-09-15 11:50:33 +00:00
Paul Berry a7cebc7349 Shared type analysis for patterns: Refactor VariableBindings logic.
This change refactors the logic for detecting overlapping and missing
variable patterns so that it can be invoked prior to the rest of type
analysis, rather than during it.  In addition separating concerns
nicely (since no types are involved in these checks), I believe this
will facilitate integration with the analyzer and front end, by
allowing them to detect these errors and find the unique set of
variables defined by a pattern, at the time they are resolving
identifiers to their corresponding declarations.

Change-Id: I40879fca46d39e78a60813db007983e57a3aec31
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259021
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-13 17:07:48 +00:00
Paul Berry 466494f5be Shared type analysis: API adjustments for analyzer and CFE
I've begun prototyping what it might look like to integrate the
current shared type analysis functionality with the analyzer and CFE,
and I've discovered some API improvements that are needed:

- The shared logic now handles the possibility that switch cases that
  share a body have been merged prior to type analysis (because the
  CFE merges them during parsing), in addition to the pre-existing
  functionality which assumed that switch case merging had to be done
  in the shared logic.

- The shared logic now returns several pieces of information as the
  result of a call to `analyzeSwitchStatement`: whether the switch
  statement had a `default` clause, whether it was exhaustive, whether
  the last case body terminates, and the type of the scrutinee.  These
  are all needed by the CFE.

- The shared logic now allows `TypeAnalyzer.errors` to be `null`,
  indicating that no errors should be reported.  This reflects how
  errors are suppressed during top level inference in the CFE.

- If a switch case lacks a `when` clause, this is reported by calling
  `handleNoWhen` rather than passing a boolean to `handleCaseHead`.

- The shared logic now reports the appropriate error when a case
  constant doesn't properly match the scrutinee's static type.

- Information about case labels is now delivered to flow analysis via
  `switchStatement_endAlternatives` rather than
  `switchStatement_beginCase`.  This made it possible to rewrite the
  shared `analyzeSwitchStatement` method in a way that requires less
  bookkeeping, because it no longer has to peek ahead to look for
  labels associated with a given case body.

- `TypeAnalyzer.analyzeExpression` is now responsible for
  understanding that "no context" and a context of `dynamic` should
  both be coalesced to `?`.  The analyzer does this (although it's not
  100% why), and it's definitely "business logic" that eventually
  belongs in the shared type analyzer.

- `TypeAnalyzer.analyzeSwitchExpression` and
  `TypeAnalyzer.analyzeSwitchStatement` no longer receive a list of
  ExpressionCaseInfo / StatementCaseInfo objects describing the cases;
  instead they query for them using a callback.  This reduces the
  lifetime of the ExpressionCaseInfo / StatementCaseInfo objects.  In
  the future, when we have record support, we could replace these
  objects with records, which would then be passed on the stack,
  avoiding any allocations.

- A new hook, `handleSwitchScrutinee`, is called right after visiting
  the "scrutinee" expression of a switch expression or switch
  statement.  This hook is needed by the analyzer to compute
  exhaustiveness.  In a future CL, I hope to move exhaustiveness
  analysis into the shared code as well, which should make this hook
  unnecessary.

- `TypeAnalyzer.analyzeSwitchStatement` now reports an error if a
  switch case completes normally and pattern support is not enabled.

- The test class `_MiniAstTypeAnalyzer` no longer overrides
  `analyzeExpression` to provide a default context type; instead,
  every call to `analyzeExpression` that didn't previously provide a
  context now provides a context of `?`.  Note that not all of these
  are correct, but they are close enough for the unit tests we have
  today.  I plan to fix them in future CLs as I replace this logic
  with shared logic.

- The hook `handleVariablePattern` is now always provided with a
  static type.  Previously, it was only provided with a static type if
  this was the first time the variable was bound in the pattern.

Change-Id: I70e3c5468312a9329fcf4ad2e13749a32d2418e7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257487
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-09-12 13:55:37 +00:00
Paul Berry 5a65ed7243 Shared type analysis: rework docs and add tests
Update the documentation for TypeAnalyzer and related classes so that
they explain the behavior of each method in terms of its effect on a
stack.  (The test logic already contained an implementation of such a
stack; production clients may or may not need to keep a stack
depending on their requirements).  This should make it more
straightforward to write clients of TypeAnalyzer.

Also, beef up the test logic in flow_analysis_mini_ast.dart and so that:

- In the event of a test error, a source location is shown, so that
  it's easy to debug and/or update the test.  This source location is
  obtained by parsing `StackTrace.current`, an approach which I
  wouldn't recommend for production code, but which is servicable for
  these low-level tests.

- Items that are popped off the stack are checked to make sure they
  have the expected kind.

The kind-checking caught a minor flaw in the previous test logic: it
was failing to distinguish expressions from expression statements.
This as been corrected, and as a result, a few test expectations in
type_inference_test.dart needed to be updated.

Change-Id: I0a2b257f6e970478c0c8e1b663dd5a367e7f24ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257486
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-09-07 23:27:03 +00:00
Paul Berry 3095c4542c Shared type analysis: check refutability and report static types.
This change adds logic to compute whether a pattern is refutable or
irrefutable, and reports the error `` if a refutable pattern is used
in an irrefutable context.

Additionally, the methods `handleConstOrLiteralPattern` and
`handleVariablePattern` are adjusted so that they report the static
type of the matcher back to the client.

A lot of internal type analysis logic previously referred to the
static type of a pattern as its "inferred type"; this nomenclature is
corrected to match the spec.

Change-Id: Icaa1118d1da41b28bea2b4f14c47578dacd85807
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256641
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-08-29 21:04:25 +00:00
Paul Berry e6caafcc3a Shared type analysis: Make switch vars non-final.
It's been decided that variables declared in switch cases should be
non-final, so we need to adjust the shared type analysis logic
accordingly.  See https://github.com/dart-lang/language/issues/2416.

Bug: https://github.com/dart-lang/language/issues/2416
Change-Id: Ie5e64ffebbd4ffa85e0d4477a3566844050f37af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256640
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-08-29 12:58:35 +00:00
Paul Berry bd2d261bc6 Shared type analysis: add support for when clauses and fix label support.
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>
2022-08-29 03:27:53 +00:00
Paul Berry ac86c134e9 Prototype shared type analysis for switches and variable patterns.
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>
2022-08-29 03:03:53 +00:00
Paul Berry 7fb9540f09 Initial infrasturucte for sharing type analysis logic.
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>
2022-08-25 18:21:05 +00:00
Lasse R.H. Nielsen 8a883fa54d Change : to = for default values in pkg.
Leaves some in parser test:
 pkg/front_end/parser_testcases/error_recovery/keyword_named_formal_parameter_prime.dart

TEST=Refactoring, covered by existing tests.

Change-Id: I7a83ef95df3cbd283878b3685b5c747bd89a1b16
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256125
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
2022-08-24 15:57:16 +00:00
Paul Berry 61e714b2fc Create ExpressionTypeAnalysisResult interface and use in tests.
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>
2022-08-23 17:04:40 +00:00
Paul Berry 2b06ac466d Flow analysis: rework some testing logic in preparation for pattern support.
- 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>
2022-08-22 22:59:26 +00:00
Paul Berry b7567b1799 Flag additional code as unreachable due to types Null and Never.
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>
2022-08-22 16:50:19 +00:00
Paul Berry e0cebd838e Flow analysis: handle promotable field accessed on a non-promotable field
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>
2022-08-19 19:38:05 +00:00
Chloe Stefantsova 1611fe6f45 [cfe] Separate out IntersectionType from TypeParameterType
TEST=Covered by existing tests

Change-Id: Ie7b99b1c109edff5198cfbf5d22e1cfb1dc130d2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/253665
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
2022-08-18 08:47:29 +00:00
Paul Berry f1043ad932 Flow analysis: discard property promotions after writes/captures.
This change ensures that when a variable `x` is written to or
captured, promotions of its fields (e.g. `x.y`, `x.y.z`, etc.) are
cancelled.  This is necessary for soundness of field promotion.

There is no effect on production code, since field promotion is not
yet enabled.

Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: Ic0739ca80cc2afe6188ada6209cb558d8cea9b63
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254620
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-08-11 20:14:38 +00:00
Paul Berry ac08c03dc6 Flow analysis: break up libraries.
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>
2022-08-11 13:18:52 +00:00
Paul Berry e6c805a3c5 Flow analysis: split test harness class.
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>
2022-08-10 20:47:41 +00:00
Paul Berry d1dcbeb973 Flow analysis: move _promotableFields out of PromotionKeyStore.
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>
2022-08-08 18:18:10 +00:00
Paul Berry 99616e9da0 Flow analysis: create test harness using a setUp() call.
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>
2022-08-08 16:58:21 +00:00
Anis Alibegić 40e18905f2 Fixed various typos in a lot of files
Closes https://github.com/dart-lang/sdk/pull/49478

TEST=Manual

GitOrigin-RevId: f4c9c6869dfe73639295e86574a021523b3d374d
Change-Id: I134a97caed4eec59d70e9cbca16b7e9a472cf2c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251902
Reviewed-by: Michael Thomsen <mit@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Kevin Chisholm <kevinjchisholm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
2022-07-25 12:21:59 +00:00
Paul Berry 2a05924ab3 Flow analysis: core implementation of field promotion.
This includes just the shared logic to track promotion of fields if
they are promotable.  It does not include the logic for figuring out
which fields are promotable, nor does it include the additional
restrictions we need to implement in order to make field promotion
sound.  Those will be addressed in other CLs.

Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I031a4272035938fd0b1b1103438ebc9642b2cb51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250347
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2022-07-18 15:07:24 +00:00
Paul Berry 247f8449d8 Flow analysis: improve debuggability of _CheckReachable in unit tests.
This improvement is helping me debug my code during the development of
field promotion.

Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I3a1a9334b7a3d094c48063cf39b4d22427ca2571
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251281
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
2022-07-12 17:27:02 +00:00
Paul Berry 8aafcbfe80 Flow analysis: remove Reference type hierarchy.
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>
2022-07-12 15:36:42 +00:00
Paul Berry e7deece1fb Flow analysis: use an integer key for FlowModel.variableInfo.
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>
2022-07-11 17:52:41 +00:00
Paul Berry 99919c69ba Flow analysis: Separate variable and type operations.
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>
2022-07-01 14:13:20 +00:00
Paul Berry 1b31046363 Change SDK constraint for _fe_analyzer_shared to 2.17.0.
This allows the _fe_analyzer_shared package to make use of the new
"super parameters" feature.

Change-Id: Iebea526cd91563a0ba603796d2df8af633adfaae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250180
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
2022-06-29 19:31:55 +00:00
Jake Macdonald e43aad1984 This is a precursor CL to a client side only InterfaceType class, which requires the ability to look at type parameters for arbitrary types in the program, in the declaration phase.
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>
2022-06-28 14:02:12 +00:00
Paul Berry d9152974ed Import exhaustiveness checker prototype into _fe_analyzer_shared.
This was developed by rynstrom@google.com as part of the patterns
feature proposal.  Moving it into _fe_analyzer_shared will allow us to
evolve it into the final implementation.

Change-Id: I31bd88fd21e1de60205363c7e6f4c405b8e6c2ab
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244761
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
2022-05-17 17:46:39 +00:00
Jake Macdonald 9846fe2621 Add FunctionTypeParameter and Parameter classes
Bug: https://github.com/dart-lang/language/issues/2208
Change-Id: I2a9bed6cc243eef2bfa313f7280d1d60c51a7b36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242140
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
2022-04-22 19:20:19 +00:00
Jake Macdonald aa1596735a support Set instances in macro arguments, add some more testing of various supported Argument types
Change-Id: I0d61a6d88dba41421fe8f3cbd3e14c62bbfa4843
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241682
Auto-Submit: Jake Macdonald <jakemac@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
2022-04-19 20:18:38 +00:00
Paul Berry ff84166410 Ensure that deferred function literals are visited in source order during each type inference stage.
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>
2022-04-12 15:24:22 +00:00
Paul Berry ccb579070a Nomenclature change: closure -> function literal (for issue 731).
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>
2022-04-12 13:04:22 +00:00
Paul Berry cd005ecdd4 Rework ClosureDependencies to include all arguments in dependency analysis.
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>
2022-04-11 16:51:01 +00:00
Jake Macdonald 1f543bcedf Refactor how actual executor instances are created.
- 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>
2022-04-05 19:25:13 +00:00
Konstantin Shcheglov 381422c05e Add RequestChannel, to be used in analyzer and frontend_server.
Change-Id: I38ec3b8b86fb0c8c8295daada48b3f6cf8e02a98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239734
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
2022-04-04 22:15:22 +00:00
Jake Macdonald ef413b95e0 add a socket communication option for the process executor
Change-Id: I0bf2ca9c8737ec0e0035e1e8e42a5e3aa3ea813c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239581
Reviewed-by: Jens Johansen <jensj@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
2022-03-31 16:35:05 +00:00
Paul Berry 05f50c7d44 Add a piece of analyzer/CFE shared infractructure for language issue 731.
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>
2022-03-30 17:28:34 +00:00
Paul Berry 4b47910994 Change DependencyWalker.walk to avoid walking already-evaluated nodes.
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>
2022-03-29 20:34:49 +00:00
Paul Berry c2c1cdc153 Move DependencyWalker from analyzer into _fe_analyzer_shared.
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>
2022-03-28 12:47:10 +00:00