commit 4e5144ac9b7b0907aa77c8f4fbb83d8fc8025e3f
author Paul Berry <paulberry@google.com> Wed Dec 11 01:39:34 2024 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Dec 11 01:39:34 2024 +0000
tree 80dd9102628e30c1723835c83395c1f6759fa708
parent ebfcd436da65802a2b20d415afe600b51e432305

[front end] Rework null shorting to use shared infrastructure.

Rework the implementation of null shorting in the front end to make
use of the shared infrastructure implemented in
https://dart-review.googlesource.com/c/sdk/+/399480.

The shared infrastructure keeps track of null-aware guards on a stack
(owned by `NullShortingMixin`), so the class
`NullAwareExpressionInferenceResult` is no longer necessary. And the
plumbing that used to be necessary to pass around linked lists of
null-aware guards is no longer needed. Also, several members of
`ExpressionInferenceResult` are no longer needed (`nullAwareGuards`,
`nullAwareAction`, `nullAwareActionType`, and `stopShorting`).

Some code remains in the front end but had to be moved:

- The logic to promote the synthetic temporary variable to
  non-nullable was moved from the `NullAwareGuard` constructor to
  `InferenceVisitorImpl.createNullAwareGuard`. This was necessary to
  ensure that the promotions are done in the correct order (first the
  shared method `startNullShorting` promotes the expression to the
  left of the `?.`, and then
  `InferenceVisitorImpl.createNullAwareGuard` promotes the synthetic
  temporary variable).

- The logic for desugaring a null-aware cascade expression is now
  implemented directly in `visitCascade`, rather than taking advantage
  of the shared infrastructure for null shorting. The rationale for
  this is twofold:

  - Null-aware cascades don't fully participate in null-shorting,
    because cascade sections are greedily parsed, so it’s impossible
    for a cascade section to be followed by any selectors that might
    continue the null shorting. So trying to re-use the shared null
    shorting infrastructure for cascade expressions would be overkill.

  - The way the front end lowers a null-aware cascade is not ideal
    (`x?..f()` is lowered to `let tmp = x in x == null ? null :
    BlockExpression({ tmp.f(); }, tmp)`, which has the disadvantage
    that it's not obvious to back-end optimization passes that the
    value of the cascade expression is equal to the value of the
    temporary variable). Keeping the logic for null-aware cascades
    separate from the logic for null shorting will make it easier to
    experiment with better lowerings in the future.

Also, since the front end doesn't always use the shared method
`analyzeExpression` for analyzing subexpressions, the shared logic for
null shorting in `analyzeExpression` was replicated in
`InferenceVisitorImpl.inferExpression`. In a future CL I would like to
change the front end to always use the shared method
`analyzeExpression` for analyzing subexpressions, so
`InferenceVisitorImpl.inferExpression` won't be needed.  But that's
not possible right now, because `InferenceVisitorImpl.inferExpression`
has behaviors that aren't implemented in the shared method
`analyzeExpression` yet (see the optional parameters `isVoidAllowed`
and `forEffect`).

Change-Id: I4d11e373bb87c3c51bcaf445880d1bffbb5c0b22
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398120
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>