tree af577af7a029542a1e2969c8a11fd63218600101
parent 272a10853dea3d94100058464f1d8d8daacbfdec
author Paul Berry <paulberry@google.com> 1662990937 +0000
committer Commit Bot <commit-bot@chromium.org> 1662990937 +0000

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>