commit 7311fd937ad50e8ab1855233486e9b68c2c2bd6d
author Paul Berry <paulberry@google.com> Wed Dec 11 01:39:34 2024 +0000
committer dart-internal-monorepo <dart-internal-monorepo@dart-ci-internal.iam.gserviceaccount.com> Tue Dec 10 17:41:37 2024 -0800
tree f19b7f507cdf1e4f79a7bc4fe10ef8251aacdbad
parent 259e8997b215cde5ac38d4ef6e53d630ef7cb307

[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>

https://dart.googlesource.com/sdk/+/4e5144ac9b7b0907aa77c8f4fbb83d8fc8025e3f