From 11929b6f68132d19872f2a43568f334b22deff36 Mon Sep 17 00:00:00 2001
From: Daco Harkes <dacoharkes@google.com>
Date: Wed, 8 Mar 2023 15:40:52 +0000
Subject: [PATCH] [vm/ffi] Change `late Finalizable` implementation #2

https://dart-review.googlesource.com/c/sdk/+/286782 omitted to treat
captured variables.

We should treat captured variables the same as other variables. We
might decide to inline the body of a closure, in which case we should
not eagerly execute initializer statements for fences.

TEST=pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart

Closes: https://github.com/dart-lang/sdk/issues/51511
Change-Id: I80744d347926087f467c561d76eca17d3a108983
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287460
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Daco Harkes <dacoharkes@google.com>
---
 .../lib/transformations/ffi/finalizable.dart  | 32 +++++++++++++------
 ...nalizable_late_initializer.dart.aot.expect |  2 +-
 .../finalizable_late_initializer.dart.expect  |  2 +-
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/pkg/vm/lib/transformations/ffi/finalizable.dart b/pkg/vm/lib/transformations/ffi/finalizable.dart
index efd00e4a627..06deefcad2c 100644
--- a/pkg/vm/lib/transformations/ffi/finalizable.dart
+++ b/pkg/vm/lib/transformations/ffi/finalizable.dart
@@ -297,7 +297,8 @@ mixin FinalizableTransformer on Transformer {
       // We can't fence late variables, they might not have been set yet.
       // Instead we fence the value variable and assign the late variable
       // value to the value variable.
-      final valueVariable = _currentScope!.lateVariableValueVariable(variable);
+      final valueVariable = _currentScope!
+          .lateVariableValueVariable(variable, checkAncestorScopes: true)!;
       final newExpression = _wrapReachabilityFences(
         expression,
         [VariableGet(valueVariable)],
@@ -725,13 +726,29 @@ ${parent?.toStringIndented(indentation: indentation + 2)}
     addDeclaration(declaration);
   }
 
-  VariableDeclaration lateVariableValueVariable(
-      VariableDeclaration lateVariable) {
+  VariableDeclaration? lateVariableValueVariable(
+      VariableDeclaration lateVariable,
+      {required bool checkAncestorScopes}) {
     final resultThisScope = _lateDeclarations[lateVariable];
     if (resultThisScope != null) {
       return resultThisScope;
     }
-    return parent!.lateVariableValueVariable(lateVariable);
+    if (!checkAncestorScopes) {
+      return null;
+    }
+    return parent?.lateVariableValueVariable(lateVariable,
+        checkAncestorScopes: checkAncestorScopes);
+  }
+
+  VariableDeclaration variableToFence(VariableDeclaration declaration,
+      {required bool checkAncestorScopes}) {
+    final possibleValueToFence = lateVariableValueVariable(declaration,
+        checkAncestorScopes: checkAncestorScopes);
+    if (possibleValueToFence != null) {
+      return possibleValueToFence;
+    }
+
+    return declaration;
   }
 
   /// Whether [allDeclarations] is empty.
@@ -806,13 +823,10 @@ ${parent?.toStringIndented(indentation: indentation + 2)}
     return [
       if (declaresThis || _capturesThis) ThisExpression(),
       for (var d in _declarations)
-        if (_lateDeclarations.containsKey(d))
-          VariableGet(_lateDeclarations[d]!)
-        else
-          VariableGet(d),
+        VariableGet(variableToFence(d, checkAncestorScopes: false)),
       if (captures != null)
         for (var d in captures.entries.where((e) => e.value).map((e) => e.key))
-          VariableGet(d),
+          VariableGet(variableToFence(d, checkAncestorScopes: true)),
     ];
   }
 
diff --git a/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.aot.expect b/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.aot.expect
index 76060ac7460..400710178ab 100644
--- a/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.aot.expect
+++ b/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.aot.expect
@@ -18,7 +18,7 @@ static method main() → void {
   late final self::Foo foo = #foo#initializer(){() → self::Foo};
   (() → Null {
     core::print(foo);
-    _in::reachabilityFence(foo);
+    _in::reachabilityFence(:foo:finalizableValue);
   })(){() → Null};
   self::Foo? :foo2:finalizableValue;
   function #foo2#initializer() → self::Foo
diff --git a/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.expect b/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.expect
index 687dd2e0b12..46e845a4a26 100644
--- a/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.expect
+++ b/pkg/vm/testcases/transformations/ffi/finalizable_late_initializer.dart.expect
@@ -18,7 +18,7 @@ static method main() → void {
   late final self::Foo foo = #foo#initializer(){() → self::Foo};
   (() → Null {
     core::print(foo);
-    _in::reachabilityFence(foo);
+    _in::reachabilityFence(:foo:finalizableValue);
   })(){() → Null};
   self::Foo? :foo2:finalizableValue;
   function #foo2#initializer() → self::Foo