[CFE] Expression compilation: #this doesn't have to be the first definition

In expression compilation, to support evaluating "this" in extensions
and extension types the VM sends us a "#this" parameter. The code
assumed this was given as the first variable in the list of definitions,
but that has turned out to not be the case and not something the VM has
ever promised.

This CL allows the #this to be at any place.

Bug: #61502

TEST=Manual

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

Author: jensjoha

pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart

@@ -44,7 +44,7 @@ Map<String, DartType>? createDefinitionsWithTypes(
   Map<String, DartType> completeDefinitions = {};
   for (int i = 0; i < definitions.length; i++) {
     String name = definitions[i];
-    if (isLegalIdentifier(name) || (i == 0 && isExtensionThisName(name))) {
+    if (isLegalIdentifier(name) || isExtensionThisName(name)) {
       ParsedType type = definitionTypesParsed[i];
       DartType dartType = type.createDartType(libraryIndex);
       completeDefinitions[name] = dartType;

pkg/front_end/lib/src/base/incremental_compiler.dart

@@ -32,9 +32,7 @@ import 'package:kernel/kernel.dart'
         DartType,
         DynamicType,
         Expression,
-        Extension,
         ExtensionType,
-        ExtensionTypeDeclaration,
         FunctionNode,
         InterfaceType,
         Library,
@@ -1959,19 +1957,21 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
       _ticker.logMs("Loaded library $libraryUri");
 
+      int? offsetToUse;
       Class? cls;
       if (className != null) {
         Builder? scopeMember = libraryBuilder.libraryNameSpace
             .lookup(className)
             ?.getable;
         if (scopeMember is ClassBuilder) {
           cls = scopeMember.cls;
+          offsetToUse = cls.fileOffset;
         } else {
           return null;
         }
       }
-      Extension? extension;
-      ExtensionTypeDeclaration? extensionType;
+
+      bool isExtensionOrExtensionType = false;
       String? extensionName;
       if (usedMethodName != null) {
         int indexOfDot = usedMethodName.indexOf(".");
@@ -1997,15 +1997,17 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           }
           extensionName = beforeDot;
           if (builder is ExtensionBuilder) {
-            extension = builder.extension;
+            isExtensionOrExtensionType = true;
+            offsetToUse = builder.fileOffset;
             Builder? subBuilder = builder.lookupLocalMember(afterDot)?.getable;
             if (subBuilder is MemberBuilder) {
               if (subBuilder.isExtensionInstanceMember) {
                 isStatic = false;
               }
             }
           } else if (builder is ExtensionTypeDeclarationBuilder) {
-            extensionType = builder.extensionTypeDeclaration;
+            isExtensionOrExtensionType = true;
+            offsetToUse = builder.fileOffset;
             Builder? subBuilder = builder.lookupLocalMember(afterDot)?.getable;
             if (subBuilder is MemberBuilder) {
               if (subBuilder.isExtensionTypeInstanceMember) {
@@ -2042,25 +2044,25 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           return null;
         }
       }
-      int index = 0;
       for (String name in usedDefinitions.keys) {
-        index++;
-        if (!(isLegalIdentifier(name) ||
-            ((extension != null || extensionType != null) &&
-                !isStatic &&
-                index == 1 &&
-                isExtensionThisName(name)))) {
-          lastGoodKernelTarget.loader.addProblem(
-            codeIncrementalCompilerIllegalParameter.withArgumentsOld(name),
-            // TODO: pass variable declarations instead of
-            // parameter names for proper location detection.
-            // https://github.com/dart-lang/sdk/issues/44158
-            -1,
-            -1,
-            libraryUri,
-          );
-          return null;
+        if (isLegalIdentifier(name)) continue;
+        if (isExtensionThisName(name) &&
+            !isStatic &&
+            isExtensionOrExtensionType) {
+          // Accept  #this for extensions and extension types.
+          continue;
         }
+
+        lastGoodKernelTarget.loader.addProblem(
+          codeIncrementalCompilerIllegalParameter.withArgumentsOld(name),
+          // TODO: pass variable declarations instead of
+          // parameter names for proper location detection.
+          // https://github.com/dart-lang/sdk/issues/44158
+          -1,
+          -1,
+          libraryUri,
+        );
+        return null;
       }
 
       // Setup scope first in two-step process:
@@ -2167,35 +2169,33 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       MemoryFileSystem fs = hfs.memory;
       fs.entityForUri(debugExprUri).writeAsStringSync(expression);
 
+      VariableDeclaration? extensionThis;
+
       // TODO: pass variable declarations instead of
       // parameter names for proper location detection.
       // https://github.com/dart-lang/sdk/issues/44158
       FunctionNode parameters = new FunctionNode(
         null,
         typeParameters: typeDefinitions,
-        positionalParameters: usedDefinitions.entries
-            .map<VariableDeclaration>(
-              (MapEntry<String, DartType> def) =>
-                  new VariableDeclarationImpl(def.key, type: def.value)
-                    ..fileOffset =
-                        cls?.fileOffset ??
-                        extension?.fileOffset ??
-                        extensionType?.fileOffset ??
-                        libraryBuilder.library.fileOffset,
-            )
-            .toList(),
+        positionalParameters: usedDefinitions.entries.map<VariableDeclaration>((
+          MapEntry<String, DartType> def,
+        ) {
+          VariableDeclarationImpl variable = new VariableDeclarationImpl(
+            def.key,
+            type: def.value,
+          )..fileOffset = offsetToUse ?? libraryBuilder.library.fileOffset;
+
+          if (isExtensionOrExtensionType &&
+              !isStatic &&
+              isExtensionThisName(def.key) &&
+              extensionThis == null) {
+            // The `#this` variable is special.
+            extensionThis = variable..isLowered = true;
+          }
+          return variable;
+        }).toList(),
       );
 
-      VariableDeclaration? extensionThis;
-      if ((extension != null || extensionType != null) &&
-          !isStatic &&
-          parameters.positionalParameters.isNotEmpty) {
-        // We expect the first parameter to be called #this and be special.
-        if (isExtensionThisName(parameters.positionalParameters.first.name)) {
-          extensionThis = parameters.positionalParameters.first;
-          extensionThis.isLowered = true;
-        }
-      }
       lastGoodKernelTarget.buildSyntheticLibrariesUntilBuildScopes([
         debugLibrary,
       ]);

pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml

@@ -0,0 +1,53 @@
+# Copyright (c) 2025, 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.
+
+# Definition, offset, method etc extracted by starting the VM with
+# `-DDFE_VERBOSE=true`, e.g.
+# ```
+# out/ReleaseX64/dart -DDFE_VERBOSE=true --enable-vm-service \
+# --disable-service-auth-codes --pause_isolates_on_start inputFile.dart
+# ```
+# and then issuing the expression compilation.
+#
+# Paused inside a function declaration inside an extension.
+# For this code the VM sends it in a weird order.
+
+sources: |
+  import 'dart:developer';
+
+  extension on String {
+    List<String> get foo {
+      List<String> result = [];
+      int offset = indexOf(':') + 1;
+      void bar() {
+        debugger();
+        while (offset < length) {
+          if (codeUnitAt(offset) != 32) {
+            return;
+          }
+          offset++;
+        }
+      }
+
+      bar();
+      result.add("$offset");
+      offset++;
+      return result;
+    }
+  }
+
+  void main(List<String> args) {
+    print(args[0].foo);
+  }
+
+definitions: ["offset", "#this"]
+definition_types: ["dart:core", "_Smi", "1", "0", "dart:core", "_OneByteString", "1", "0"]
+type_definitions: []
+type_bounds: []
+type_defaults: []
+method: "bar"
+static: true
+offset: 161
+scriptUri: main.dart
+expression: "offset + this.length"

pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml.expect

@@ -0,0 +1,4 @@
+Errors: {
+}
+method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr(dart.core::_Smi offset, lowered dart.core::_OneByteString #this) → dynamic
+  return offset.{dart.core::_IntegerImplementation::+}(#this.{dart.core::_StringBase::length}{dart.core::int}){(dart.core::num) → dart.core::num};

pkg/vm/lib/incremental_compiler.dart

@@ -262,7 +262,7 @@ class IncrementalCompiler {
       // Revert to old behaviour of setting everything to dynamic.
       for (int i = 0; i < definitions.length; i++) {
         String name = definitions[i];
-        if (isLegalIdentifier(name) || (i == 0 && isExtensionThisName(name))) {
+        if (isLegalIdentifier(name) || isExtensionThisName(name)) {
           completeDefinitions[name] = new DynamicType();
         }
       }