Add support for null shorting expressions to the Wolf analysis prototype.

The AST-to-IR conversion stage now handles null-shorting property
accesses (both for reads and writes). This required adding the
following instruction types: `eq`, `block`, and `brIf`.

In order to make `eq` easier to test, support was also added for
AST-to-IR conversion of testing binary expressions using `==`.

The way null shorting is encoded in the IR is by issuing a `block`
instruction when null shorting starts, and an `end` instruction when
it terminates. Anywhere a null check appears within the null shorting
expression, the null check uses a `brIf(0)` instruction to branch to
the `end` in the case a `null` is found. To make it easier to keep
track of when `block` and `end` instructions need to be generated,
`RawIRWriter` keeps track of a count of the current nesting of control
flow contsructs.

In order for the interpreter to find to the appropriate `end`
instruction when a branch is taken, a new scope analyzer is
added. Later CLs will expand on it and use it for static analysis as
well.

Change-Id: I09ca34eaa900d47f7a4014cbc8db2a48a1d69e1e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336800
Reviewed-by: Phil Quitslund <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

pkg/analyzer/lib/src/wolf/ir/ast_to_ir.dart

@@ -103,12 +103,23 @@ class _AstToIRVisitor extends ThrowingAstVisitor<_LValueTemplates> {
   _LValueTemplates dispatchLValue(Expression node) => node.accept(this)!;
 
   /// Visits [node], reporting progress to [eventListener].
-  void dispatchNode(AstNode node) {
+  ///
+  /// If [node] has null shorting behavior, then [terminateNullShorting]
+  /// determines how the null shorting behavior is handled. If
+  /// [terminateNullShorting] is `true` (the default), then null shorting will
+  /// be terminated after visiting [node], by emitting an `end` instruction;
+  /// this means that in the case where the null short occurs, the expression
+  /// will evaluate to `null`. If [terminateNullShorting] is `false`, then null
+  /// shorting won't be terminated; this means that in the case where the null
+  /// short occurs, execution of the parent node will be skipped too.
+  void dispatchNode(AstNode node, {bool terminateNullShorting = true}) {
     eventListener.onEnterNode(node);
+    var previousNestingLevel = ir.nestingLevel;
     var lValueTemplates = node.accept(this);
     // If the node was an L-value, then its visitor didn't actually perform the
     // read, so do that now.
     lValueTemplates?.simpleRead(this);
+    if (terminateNullShorting) ir.endTo(previousNestingLevel);
     eventListener.onExitNode();
   }
 
@@ -135,6 +146,37 @@ class _AstToIRVisitor extends ThrowingAstVisitor<_LValueTemplates> {
         twoArguments);
   }
 
+  /// Performs a null check that is part of a null shorting expression.
+  ///
+  /// If the value at the top of the stack is `null`, execution will branch to
+  /// the end of the null shorting expression, and the null shorting expression
+  /// will evaluate to `null`. Otherwise, execution will proceed normally.
+  ///
+  /// [previousNestingLevel] is the value returned by [RawIRWriter.nestingLevel]
+  /// at the beginning of the null shorting expression. It is used to detect
+  /// whether null shorting has already been begun, and therefore whether a
+  /// `block` instruction needs to be output.
+  void nullShortingCheck({required int previousNestingLevel}) {
+    assert(previousNestingLevel <= ir.nestingLevel);
+    // Stack: value
+    ir.dup();
+    // Stack: value value
+    ir.literal(null_);
+    // Stack: value value null
+    ir.eq();
+    // Stack: value (value == null)
+    if (previousNestingLevel == ir.nestingLevel) {
+      // Null shorting hasn't begun yet for the containing expression, so start
+      // it now by opening a block; the block will be ended at the end of the
+      // null shorting expression, so it will be the branch target for null
+      // shorts.
+      ir.block(2, 1);
+      // Stack: BLOCK(1) value (value == null)
+    }
+    ir.brIf(0);
+    // Stack: BLOCK(1)? value
+  }
+
   Null this_() {
     ir.readLocal(0); // Stack: this
   }
@@ -157,6 +199,22 @@ class _AstToIRVisitor extends ThrowingAstVisitor<_LValueTemplates> {
     }
   }
 
+  @override
+  Null visitBinaryExpression(BinaryExpression node) {
+    var tokenType = node.operator.type;
+    switch (tokenType) {
+      case TokenType.EQ_EQ:
+        dispatchNode(node.leftOperand);
+        // Stack: lhs
+        dispatchNode(node.rightOperand);
+        // Stack: lhs rhs
+        ir.eq();
+      // Stack: (lhs == rhs)
+      default:
+        throw UnimplementedError('TODO(paulberry): $node');
+    }
+  }
+
   @override
   Null visitBlock(Block node) {
     var previousLocalVariableCount = ir.localVariableCount;
@@ -271,10 +329,14 @@ class _AstToIRVisitor extends ThrowingAstVisitor<_LValueTemplates> {
 
   @override
   _LValueTemplates visitPropertyAccess(PropertyAccess node) {
-    // TODO(paulberry): handle null shorting
+    var previousNestingLevel = ir.nestingLevel;
     // TODO(paulberry): handle cascades
-    dispatchNode(node.target!);
+    dispatchNode(node.target!, terminateNullShorting: false);
     // Stack: target
+    if (node.isNullAware) {
+      nullShortingCheck(previousNestingLevel: previousNestingLevel);
+    }
+    // Stack: BLOCK(1)? target
     return _PropertyAccessTemplates(node.propertyName);
   }
 
@@ -395,8 +457,6 @@ class _LocalTemplates extends _LValueTemplates {
 /// subexpression values in order to read or write the L-value.  These methods
 /// are abstract, and are defined in a derived class for each specific kind of
 /// L-value supported by Dart.
-///
-// TODO(paulberry): add null shorting support.
 sealed class _LValueTemplates {
   /// Outputs the IR instructions for a simple read of the L-value.
   ///
@@ -414,8 +474,6 @@ sealed class _LValueTemplates {
 }
 
 /// Instruction templates for converting a property access to IR.
-///
-// TODO(paulberry): handle null shorting
 class _PropertyAccessTemplates extends _LValueTemplates {
   final SimpleIdentifier property;
 

pkg/analyzer/lib/src/wolf/ir/interpreter.dart

@@ -6,6 +6,7 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/wolf/ir/call_descriptor.dart';
 import 'package:analyzer/src/wolf/ir/coded_ir.dart';
 import 'package:analyzer/src/wolf/ir/ir.dart';
+import 'package:analyzer/src/wolf/ir/scope_analyzer.dart';
 import 'package:meta/meta.dart';
 
 /// Evaluates [ir], passing in [args], and returns the result.
@@ -22,14 +23,35 @@ import 'package:meta/meta.dart';
 /// that an instruction sequence behaves as it's expected to.
 @visibleForTesting
 Object? interpret(CodedIRContainer ir, List<Object?> args,
-    {required CallHandler Function(CallDescriptor) callDispatcher}) {
-  return _IRInterpreter(ir, callDispatcher: callDispatcher).run(args);
+    {required Scopes scopes, required CallDispatcher callDispatcher}) {
+  return _IRInterpreter(ir, scopes: scopes, callDispatcher: callDispatcher)
+      .run(args);
 }
 
 /// Function type invoked by [interpret] to execute a `call` instruction.
 typedef CallHandler = Object? Function(
     List<Object?> positionalArguments, Map<String, Object?> namedArguments);
 
+/// Interface used by [interpret] to query the behavior of calls to external
+/// code.
+abstract interface class CallDispatcher {
+  /// Evaluates a call to `operator==`, using virtual dispatch on [firstValue],
+  /// and passing [secondValue] as the parameter to `operator==`.
+  ///
+  /// In accordance with Dart semantics, this method is only called if both
+  /// [firstValue] and [secondValue] are non-null.
+  bool equals(Object firstValue, Object secondValue);
+
+  /// Looks up the function that can be used to evaluate calls to
+  /// [callDescriptor].
+  ///
+  /// The interpreter may invoke this method for any [CallDescriptor] in the
+  /// IR's call descriptor table (whether or not it's invoked), and it may cache
+  /// the results. However, it is guaranteed to call the [CallHandler] exactly
+  /// once for each `call` instruction that is interpreted.
+  CallHandler lookupCallDescriptor(CallDescriptor callDescriptor);
+}
+
 /// Interpreter representation of a heap object.
 ///
 /// This class should not be used for the types [int], [double], [String], or
@@ -62,29 +84,101 @@ class SoundnessError extends Error {
       'Soundness error at $address ($instructionString): $message';
 }
 
+/// An entry on the control flow stack, representing a control flow construct
+/// (such as a `block`) that the interpreter is currently executing.
+class _ControlFlowStackEntry {
+  /// The index into [_IRInterpreter.stack] before the first input to control
+  /// flow construct.
+  ///
+  /// This is called a "fence" because it represents the dividing line between
+  /// stack values that belong to the instructions inside the control flow
+  /// construct and stack values that belong to the instructions outside the
+  /// control flow construct. If a branch instruction targets the control flow
+  /// construct, this helps to determine which stack values should be discarded
+  /// (see [outputCount]).
+  final int stackFence;
+
+  /// The length of [_IRInterpreter.locals] at the time the control flow
+  /// construct was entered.
+  ///
+  /// This is called a "fence" because it represents the dividing line between
+  /// locals that belong to the instructions inside the control flow construct
+  /// and locals that belong to the instructions outside the control flow
+  /// construct. If a branch instruction targets the control flow construct,
+  /// then locals whose index is greater than equal to this value will
+  /// automatically be released.
+  final int localFence;
+
+  /// The number of outputs of the control flow construct.
+  ///
+  /// If a branch instruction targets the control flow construct, this is the
+  /// number of entries at the top of [_IRInterpreter.stack] that will remain on
+  /// the stack after the branch is taken. Any other stack entries belonging to
+  /// the instructions inside the control flow construct will be discarded (see
+  /// [stackFence]).
+  final int outputCount;
+
+  /// The scope index (as defined by [Scopes]) corresponding to the instructions
+  /// that delimit the control flow construct.
+  final int scope;
+
+  _ControlFlowStackEntry(
+      {required this.stackFence,
+      required this.localFence,
+      required this.outputCount,
+      required this.scope});
+}
+
 class _IRInterpreter {
+  static const keepGoing = _KeepGoing();
   final CodedIRContainer ir;
+  final Scopes scopes;
+  final CallDispatcher callDispatcher;
   final List<CallHandler> callHandlers;
   final stack = <Object?>[];
   final locals = <_LocalSlot>[];
+  final controlFlowStack = <_ControlFlowStackEntry>[];
   var address = 1;
 
-  _IRInterpreter(this.ir,
-      {required CallHandler Function(CallDescriptor) callDispatcher})
-      : callHandlers = ir.mapCallDescriptors(callDispatcher);
+  /// The scope index (as defined by [Scopes]) corresponding to the last begin
+  /// instruction preceding [address].
+  var mostRecentScope = 0;
+
+  _IRInterpreter(this.ir, {required this.scopes, required this.callDispatcher})
+      : callHandlers =
+            ir.mapCallDescriptors(callDispatcher.lookupCallDescriptor);
 
   /// Performs the necessary logic for a `br`, `brIf`, or `brIndex` instruction.
   ///
   /// [nesting] indicates which enclosing control flow construct is targeted by
   /// the branch (where 0 means the innermost).
   ///
-  /// The returned value is the value that should be returned to the caller.
+  /// The returned value is either:
+  /// - [keepGoing], indicating that interpretation should continue from the
+  ///   instruction following [address], or
+  /// - Some other value, indicating that the code being interpreted has
+  ///   finished executing, and this value should be returned to the caller.
   Object? branch(int nesting) {
-    if (nesting != 0) {
-      throw UnimplementedError('TODO(paulberry): nonzero branch nesting');
+    while (nesting-- > 0) {
+      controlFlowStack.removeLast();
+    }
+    if (controlFlowStack.isNotEmpty) {
+      var stackEntry = controlFlowStack.removeLast();
+      var stackFence = stackEntry.stackFence;
+      var outputCount = stackEntry.outputCount;
+      var newStackLength = stackFence + outputCount;
+      stack.setRange(stackFence, newStackLength, stack,
+          stack.length - stackEntry.outputCount);
+      stack.length = stackFence + outputCount;
+      locals.length = stackEntry.localFence;
+      var scope = stackEntry.scope;
+      address = scopes.endAddress(scope);
+      mostRecentScope = scopes.lastDescendant(scope);
+      return keepGoing;
+    } else {
+      // Branch targets the function, so return from the code being interpreted.
+      return stack.last;
     }
-    // Branch targets the function, so return from the code being interpreted.
-    return stack.last;
   }
 
   Object? run(List<Object?> args) {
@@ -98,15 +192,37 @@ class _IRInterpreter {
     }
     stack.addAll(args);
     while (true) {
+      assert(scopes.mostRecentScope(address - 1) == mostRecentScope);
       switch (ir.opcodeAt(address)) {
         case Opcode.alloc:
           var count = Opcode.alloc.decodeCount(ir, address);
           for (var i = 0; i < count; i++) {
             locals.add(_LocalSlot());
           }
+        case Opcode.block:
+          var inputCount = Opcode.block.decodeInputCount(ir, address);
+          var outputCount = Opcode.block.decodeOutputCount(ir, address);
+          var scope = ++mostRecentScope;
+          assert(scopes.beginAddress(scope) == address);
+          controlFlowStack.add(_ControlFlowStackEntry(
+              stackFence: stack.length - inputCount,
+              localFence: locals.length,
+              outputCount: outputCount,
+              scope: scope));
         case Opcode.br:
           var nesting = Opcode.br.decodeNesting(ir, address);
-          return branch(nesting);
+          var result = branch(nesting);
+          if (!identical(result, keepGoing)) {
+            return result;
+          }
+        case Opcode.brIf:
+          var nesting = Opcode.brIf.decodeNesting(ir, address);
+          if (stack.removeLast() as bool) {
+            var result = branch(nesting);
+            if (!identical(result, keepGoing)) {
+              return result;
+            }
+          }
         case Opcode.call:
           var argumentNames = ir.decodeArgumentNames(
               Opcode.call.decodeArgumentNames(ir, address));
@@ -130,8 +246,27 @@ class _IRInterpreter {
         case Opcode.dup:
           stack.add(stack.last);
         case Opcode.end:
-          assert(stack.length == 1);
-          return stack.last;
+          if (controlFlowStack.isEmpty) {
+            assert(stack.length == 1);
+            return stack.last;
+          } else {
+            var stackEntry = controlFlowStack.last;
+            assert(
+                stack.length == stackEntry.stackFence + stackEntry.outputCount);
+            assert(locals.length == stackEntry.localFence);
+            // Continue with the code following the block.
+            controlFlowStack.removeLast();
+          }
+        case Opcode.eq:
+          var secondValue = stack.removeLast();
+          var firstValue = stack.removeLast();
+          if (firstValue == null) {
+            stack.add(null == secondValue);
+          } else if (secondValue == null) {
+            stack.add(false);
+          } else {
+            stack.add(callDispatcher.equals(firstValue, secondValue));
+          }
         case Opcode.literal:
           var value = Opcode.literal.decodeValue(ir, address);
           stack.add(ir.decodeLiteral(value));
@@ -173,6 +308,12 @@ class _IRInterpreter {
       message: message);
 }
 
+/// Sentinel value used by [_IRInterpreter.branch] to indicate that the
+/// interpreter should keep executing instructions.
+class _KeepGoing {
+  const _KeepGoing();
+}
+
 /// Storage for a single local variable.
 class _LocalSlot {
   /// The contents of the local variable, or [_NoValue] if the slot is empty.