[cfe] Don't use late final for pattern caching

This changes the pattern matching lowering to avoid using late final variables (or lowerings thereof) and instead inline the initializer where used. This avoids generating closures that are hard for backends to reason about and optimize.

Closes #52805
Closes #53804

Change-Id: Ia887446dd4d4475d05cf852c812bfe4b851de261
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338860
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>

Author: johnniwinther

pkg/front_end/lib/src/fasta/type_inference/matching_cache.dart

@@ -1047,12 +1047,6 @@ class Cache {
   /// Otherwise [_isSetVariable] is unused.
   VariableDeclaration? _isSetVariable;
 
-  /// If cached using late lowering, this will be the variable for the local
-  /// function that initializes or reads [_variable].
-  ///
-  /// Otherwise [_getVariable] is unused.
-  VariableDeclaration? _getVariable;
-
   /// The name used to name [_variable], [_isSetVariable] and [_getVariable].
   final String _name;
 
@@ -1128,123 +1122,27 @@ class Cache {
           .createExpression(typeEnvironment, inCacheInitializer: false);
     } else {
       CacheExpression cacheableExpression = _accesses[accessKey]!;
-      if (_accesses.length == 1) {
+      if (!_isLate) {
+        // To avoid closurizing initializers we always inline the initializer
+        // unless it is not a late variable.
         VariableDeclaration? variable = _variable;
-        VariableDeclaration? isSetVariable = _isSetVariable;
         if (variable == null) {
-          DartType type = cacheableExpression.getType(typeEnvironment);
-          if (_matchingCache.useLowering && _isLate) {
-            variable = _variable =
-                createUninitializedVariable(type, fileOffset: _fileOffset)
-                  ..name = _name;
-            _matchingCache.registerDeclaration(variable);
-            isSetVariable = _isSetVariable = createInitializedVariable(
-                createBoolLiteral(false, fileOffset: _fileOffset),
-                typeEnvironment.coreTypes.boolNonNullableRawType,
-                fileOffset: _fileOffset)
-              ..name = '$_name#isSet';
-            _matchingCache.registerDeclaration(isSetVariable);
-            VariableDeclaration getVariable =
-                _getVariable = createUninitializedVariable(
-                    new FunctionType([], type, Nullability.nonNullable),
-                    fileOffset: _fileOffset)
-                  ..name = '$_name#func'
-                  ..isFinal = true;
-
-            Statement body;
-            if (_matchingCache.useVerboseEncodingForDebugging) {
-              body = createBlock([
-                createIfStatement(
-                    createNot(createVariableGet(isSetVariable)),
-                    createBlock([
-                      createExpressionStatement(createStaticInvocation(
-                          typeEnvironment.coreTypes.printProcedure,
-                          createArguments([
-                            createStringConcatenation([
-                              createStringLiteral('compute $_name',
-                                  fileOffset: _fileOffset),
-                            ], fileOffset: _fileOffset)
-                          ], fileOffset: _fileOffset),
-                          fileOffset: _fileOffset)),
-                      createExpressionStatement(createVariableSet(isSetVariable,
-                          createBoolLiteral(true, fileOffset: _fileOffset),
-                          fileOffset: _fileOffset)),
-                      createExpressionStatement(createVariableSet(
-                          variable,
-                          cacheableExpression.expression.createExpression(
-                              typeEnvironment,
-                              inCacheInitializer: true),
-                          fileOffset: _fileOffset)),
-                    ], fileOffset: _fileOffset),
-                    fileOffset: _fileOffset),
-                createExpressionStatement(createStaticInvocation(
-                    typeEnvironment.coreTypes.printProcedure,
-                    createArguments([
-                      createStringConcatenation([
-                        createStringLiteral('$_name = ',
-                            fileOffset: _fileOffset),
-                        createVariableGet(variable)
-                      ], fileOffset: _fileOffset)
-                    ], fileOffset: _fileOffset),
-                    fileOffset: _fileOffset)),
-                createReturnStatement(createVariableGet(variable),
-                    fileOffset: _fileOffset),
-              ], fileOffset: _fileOffset)
-                ..fileOffset = _fileOffset;
-            } else {
-              body = createReturnStatement(
-                  createConditionalExpression(
-                      createVariableGet(isSetVariable),
-                      createVariableGet(variable),
-                      createLetEffect(
-                          effect: createVariableSet(isSetVariable,
-                              createBoolLiteral(true, fileOffset: _fileOffset),
-                              fileOffset: _fileOffset),
-                          result: createVariableSet(
-                              variable,
-                              cacheableExpression.expression.createExpression(
-                                  typeEnvironment,
-                                  inCacheInitializer: true),
-                              fileOffset: _fileOffset)),
-                      staticType: type,
-                      fileOffset: _fileOffset),
-                  fileOffset: _fileOffset);
-            }
-            FunctionDeclaration functionDeclaration = new FunctionDeclaration(
-                    getVariable, new FunctionNode(body, returnType: type))
-                // TODO(johnniwinther): Reinsert the file offset when the vm
-                //  doesn't use it for function declaration identity.
-                /*..fileOffset = fileOffset*/;
-            getVariable.type = functionDeclaration.function
-                .computeFunctionType(Nullability.nonNullable);
-            _matchingCache.registerDeclaration(functionDeclaration);
-          } else {
-            variable = _variable = createVariableCache(
-                cacheableExpression.expression.createExpression(typeEnvironment,
-                    inCacheInitializer: true),
-                cacheableExpression.getType(typeEnvironment))
-              ..isConst = _isConst
-              ..isLate = _isLate
-              ..name = _name;
-            if (_isLate) {
-              // Avoid step debugging on the declaration of caching variables.
-              // TODO(johnniwinther): Find a more systematic way of omitting
-              // offsets for better step debugging.
-              variable.fileOffset = TreeNode.noOffset;
-            }
-            _matchingCache.registerDeclaration(variable);
+          variable = _variable = createVariableCache(
+              cacheableExpression.expression
+                  .createExpression(typeEnvironment, inCacheInitializer: true),
+              cacheableExpression.getType(typeEnvironment))
+            ..isConst = _isConst
+            ..isLate = _isLate
+            ..name = _name;
+          if (_isLate) {
+            // Avoid step debugging on the declaration of caching variables.
+            // TODO(johnniwinther): Find a more systematic way of omitting
+            // offsets for better step debugging.
+            variable.fileOffset = TreeNode.noOffset;
           }
+          _matchingCache.registerDeclaration(variable);
         }
-        if (_matchingCache.useLowering && _isLate) {
-          LocalFunctionInvocation localFunctionInvocation =
-              createLocalFunctionInvocation(_getVariable!,
-                  fileOffset: _fileOffset)
-                ..fileOffset = TreeNode.noOffset;
-          localFunctionInvocation.arguments.fileOffset = TreeNode.noOffset;
-          result = localFunctionInvocation;
-        } else {
-          result = createVariableGet(variable)..fileOffset = TreeNode.noOffset;
-        }
+        result = createVariableGet(variable)..fileOffset = TreeNode.noOffset;
       } else {
         assert(_isLate, "Unexpected non-late cache ${cacheKey.name}");
 

pkg/front_end/test/spell_checking_list_code.txt

@@ -247,6 +247,7 @@ cli
 client's
 clil
 clock
+closurizing
 cls
 cm
 cmdline

pkg/front_end/testcases/dart2js/issue52805.dart

@@ -0,0 +1,14 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+use(o) {}
+
+void Function() f(List<int> list) {
+  return switch (list) {
+    [final item] => () {
+        use(item);
+      },
+    [...] => () {},
+  };
+}

pkg/front_end/testcases/dart2js/issue52805.dart.strong.expect

@@ -0,0 +1,33 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method use(dynamic o) → dynamic {}
+static method f(core::List<core::int> list) → () → void {
+  return block {
+    () → void #t1;
+    final synthesized core::List<core::int> #0#0 = list;
+    #L1:
+    {
+      {
+        final hoisted core::int item;
+        if(#0#0.{core::List::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C1 && (let final dynamic #t2 = item = #0#0.{core::List::[]}(0){(core::int) → core::int} in true)) {
+          #t1 = () → void {
+            self::use(item);
+          };
+          break #L1;
+        }
+      }
+      {
+        if(true) {
+          #t1 = () → void {};
+          break #L1;
+        }
+      }
+    }
+  } =>#t1;
+}
+
+constants  {
+  #C1 = 1
+}

pkg/front_end/testcases/dart2js/issue52805.dart.strong.transformed.expect

@@ -0,0 +1,33 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method use(dynamic o) → dynamic {}
+static method f(core::List<core::int> list) → () → void {
+  return block {
+    () → void #t1;
+    final synthesized core::List<core::int> #0#0 = list;
+    #L1:
+    {
+      {
+        final hoisted core::int item;
+        if(#0#0.{core::List::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C1 && (let final core::int #t2 = item = #0#0.{core::List::[]}(0){(core::int) → core::int} in true)) {
+          #t1 = () → void {
+            self::use(item);
+          };
+          break #L1;
+        }
+      }
+      {
+        if(true) {
+          #t1 = () → void {};
+          break #L1;
+        }
+      }
+    }
+  } =>#t1;
+}
+
+constants  {
+  #C1 = 1
+}

pkg/front_end/testcases/dart2js/issue52805.dart.textual_outline.expect

@@ -0,0 +1,2 @@
+use(o) {}
+void Function() f(List<int> list) {}

pkg/front_end/testcases/dart2js/issue52805.dart.textual_outline_modelled.expect

@@ -0,0 +1,2 @@
+use(o) {}
+void Function() f(List<int> list) {}

pkg/front_end/testcases/dart2js/issue52805.dart.weak.expect

@@ -0,0 +1,35 @@
+library;
+import self as self;
+import "dart:core" as core;
+import "dart:_internal" as _in;
+
+static method use(dynamic o) → dynamic {}
+static method f(core::List<core::int> list) → () → void {
+  return block {
+    () → void #t1;
+    final synthesized core::List<core::int> #0#0 = list;
+    #L1:
+    {
+      {
+        final hoisted core::int item;
+        if(#0#0.{core::List::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C1 && (let final dynamic #t2 = item = #0#0.{core::List::[]}(0){(core::int) → core::int} in true)) {
+          #t1 = () → void {
+            self::use(item);
+          };
+          break #L1;
+        }
+      }
+      {
+        if(true) {
+          #t1 = () → void {};
+          break #L1;
+        }
+      }
+      throw new _in::ReachabilityError::•("`null` encountered as case in a switch expression with a non-nullable type.");
+    }
+  } =>#t1;
+}
+
+constants  {
+  #C1 = 1
+}

pkg/front_end/testcases/dart2js/issue52805.dart.weak.modular.expect

@@ -0,0 +1,35 @@
+library;
+import self as self;
+import "dart:core" as core;
+import "dart:_internal" as _in;
+
+static method use(dynamic o) → dynamic {}
+static method f(core::List<core::int> list) → () → void {
+  return block {
+    () → void #t1;
+    final synthesized core::List<core::int> #0#0 = list;
+    #L1:
+    {
+      {
+        final hoisted core::int item;
+        if(#0#0.{core::List::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C1 && (let final dynamic #t2 = item = #0#0.{core::List::[]}(0){(core::int) → core::int} in true)) {
+          #t1 = () → void {
+            self::use(item);
+          };
+          break #L1;
+        }
+      }
+      {
+        if(true) {
+          #t1 = () → void {};
+          break #L1;
+        }
+      }
+      throw new _in::ReachabilityError::•("`null` encountered as case in a switch expression with a non-nullable type.");
+    }
+  } =>#t1;
+}
+
+constants  {
+  #C1 = 1
+}

pkg/front_end/testcases/dart2js/issue52805.dart.weak.outline.expect

@@ -0,0 +1,8 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+static method use(dynamic o) → dynamic
+  ;
+static method f(core::List<core::int> list) → () → void
+  ;

pkg/front_end/testcases/dart2js/issue52805.dart.weak.transformed.expect

@@ -0,0 +1,35 @@
+library;
+import self as self;
+import "dart:core" as core;
+import "dart:_internal" as _in;
+
+static method use(dynamic o) → dynamic {}
+static method f(core::List<core::int> list) → () → void {
+  return block {
+    () → void #t1;
+    final synthesized core::List<core::int> #0#0 = list;
+    #L1:
+    {
+      {
+        final hoisted core::int item;
+        if(#0#0.{core::List::length}{core::int} =={core::num::==}{(core::Object) → core::bool} #C1 && (let final core::int #t2 = item = #0#0.{core::List::[]}(0){(core::int) → core::int} in true)) {
+          #t1 = () → void {
+            self::use(item);
+          };
+          break #L1;
+        }
+      }
+      {
+        if(true) {
+          #t1 = () → void {};
+          break #L1;
+        }
+      }
+      throw new _in::ReachabilityError::•("`null` encountered as case in a switch expression with a non-nullable type.");
+    }
+  } =>#t1;
+}
+
+constants  {
+  #C1 = 1
+}

pkg/front_end/testcases/dart2js/issue53804.dart

@@ -0,0 +1,21 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+class A {
+  int? f1;
+  double? f2;
+
+  @pragma('vm:never-inline')
+  String foo() {
+    return switch ((this.f1, this.f2)) {
+      (final f1, _) => '$f1',
+      (null, final f2) => '$f2',
+      (null, null) => '?',
+    };
+  }
+}
+
+void main() {
+  print(A().foo());
+}