Skip to content

Commit

Permalink
[front end] Rework null shorting to use shared infrastructure.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
  • Loading branch information
stereotype441 authored and Commit Queue committed Dec 11, 2024
1 parent ebfcd43 commit 4e5144a
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 406 deletions.
82 changes: 1 addition & 81 deletions pkg/front_end/lib/src/type_inference/inference_results.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
import 'package:_fe_analyzer_shared/src/util/link.dart';
import 'package:kernel/ast.dart';

import '../codes/cfe_codes.dart';
Expand Down Expand Up @@ -364,19 +362,6 @@ class ExpressionInferenceResult {
{this.postCoercionType = null})
: assert(isKnown(inferredType), "$inferredType is not known.");

/// The guards used for null-aware access if the expression is part of a
/// null-shorting.
Link<NullAwareGuard> get nullAwareGuards => const Link<NullAwareGuard>();

/// If the expression is part of a null-shorting, this is the action performed
/// on the guarded variable, found as the first guard in [nullAwareGuards].
/// Otherwise, this is the same as [expression].
Expression get nullAwareAction => expression;

DartType get nullAwareActionType => inferredType;

ExpressionInferenceResult stopShorting() => this;

@override
String toString() => 'ExpressionInferenceResult($inferredType,$expression)';
}
Expand All @@ -392,20 +377,7 @@ class NullAwareGuard {
final InferenceVisitorBase _inferrer;

NullAwareGuard(
this._nullAwareVariable, this._nullAwareFileOffset, this._inferrer) {
// Ensure the initializer of [_nullAwareVariable] is promoted to
// non-nullable.
_inferrer.flowAnalysis.nullAwareAccess_rightBegin(
_nullAwareVariable.initializer,
new SharedTypeView(_nullAwareVariable.type));
// Ensure [_nullAwareVariable] is promoted to non-nullable.
// TODO(johnniwinther): Avoid creating a [VariableGet] to promote the
// variable.
VariableGet read = new VariableGet(_nullAwareVariable);
_inferrer.flowAnalysis.variableRead(read, _nullAwareVariable);
_inferrer.flowAnalysis.nullAwareAccess_rightBegin(
read, new SharedTypeView(_nullAwareVariable.type));
}
this._nullAwareVariable, this._nullAwareFileOffset, this._inferrer);

/// Creates the null-guarded application of [nullAwareAction] with the
/// [inferredType].
Expand All @@ -417,8 +389,6 @@ class NullAwareGuard {
///
Expression createExpression(
DartType inferredType, Expression nullAwareAction) {
// End non-nullable promotion of [_nullAwareVariable].
_inferrer.flowAnalysis.nullAwareAccess_end();
// End non-nullable promotion of the initializer of [_nullAwareVariable].
_inferrer.flowAnalysis.nullAwareAccess_end();
Expression equalsNull = _inferrer.createEqualsNull(
Expand All @@ -437,53 +407,3 @@ class NullAwareGuard {
String toString() =>
'NullAwareGuard($_nullAwareVariable,$_nullAwareFileOffset)';
}

/// The result of an expression inference that is guarded with a null aware
/// variable.
class NullAwareExpressionInferenceResult implements ExpressionInferenceResult {
/// The inferred type of the expression.
@override
final DartType inferredType;

/// The inferred type of the [nullAwareAction].
@override
final DartType nullAwareActionType;

@override
final Link<NullAwareGuard> nullAwareGuards;

@override
final Expression nullAwareAction;

NullAwareExpressionInferenceResult(this.inferredType,
this.nullAwareActionType, this.nullAwareGuards, this.nullAwareAction)
: assert(nullAwareGuards.isNotEmpty);

@override
Expression get expression {
throw new UnsupportedError('Shorting must be explicitly stopped before'
'accessing the expression result of a '
'NullAwareExpressionInferenceResult');
}

@override
// Coverage-ignore(suite): Not run.
DartType? get postCoercionType => null;

@override
ExpressionInferenceResult stopShorting() {
Expression expression = nullAwareAction;
Link<NullAwareGuard> nullAwareGuard = nullAwareGuards;
while (nullAwareGuard.isNotEmpty) {
expression =
nullAwareGuard.head.createExpression(inferredType, expression);
nullAwareGuard = nullAwareGuard.tail!;
}
return new ExpressionInferenceResult(inferredType, expression);
}

@override
String toString() =>
'NullAwareExpressionInferenceResult($inferredType,$nullAwareGuards,'
'$nullAwareAction)';
}
Loading

0 comments on commit 4e5144a

Please sign in to comment.