[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: https://github.com/dart-lang/sdk/issues/61502
TEST=Manual
Change-Id: I8868a2e05704cda57f2835e9f261ade12566bf7c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451760
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart b/pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart
index a512f89..0d41e3e 100644
--- a/pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart
+++ b/pkg/front_end/lib/src/api_prototype/expression_compilation_tools.dart
@@ -44,7 +44,7 @@
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;
diff --git a/pkg/front_end/lib/src/base/incremental_compiler.dart b/pkg/front_end/lib/src/base/incremental_compiler.dart
index ba8bc57..1971aa1 100644
--- a/pkg/front_end/lib/src/base/incremental_compiler.dart
+++ b/pkg/front_end/lib/src/base/incremental_compiler.dart
@@ -32,9 +32,7 @@
DartType,
DynamicType,
Expression,
- Extension,
ExtensionType,
- ExtensionTypeDeclaration,
FunctionNode,
InterfaceType,
Library,
@@ -1959,6 +1957,7 @@
_ticker.logMs("Loaded library $libraryUri");
+ int? offsetToUse;
Class? cls;
if (className != null) {
Builder? scopeMember = libraryBuilder.libraryNameSpace
@@ -1966,12 +1965,13 @@
?.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,7 +1997,8 @@
}
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) {
@@ -2005,7 +2006,8 @@
}
}
} 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 @@
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 @@
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,
]);
diff --git a/pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml b/pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml
new file mode 100644
index 0000000..d36e550
--- /dev/null
+++ b/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"
diff --git a/pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml.expect b/pkg/front_end/testcases/expression/extension_with_inner_function_declaration_vm_sending_in_weird_order.expression.yaml.expect
new file mode 100644
index 0000000..cdb0642
--- /dev/null
+++ b/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};
diff --git a/pkg/vm/lib/incremental_compiler.dart b/pkg/vm/lib/incremental_compiler.dart
index f96cc67..92c557a 100644
--- a/pkg/vm/lib/incremental_compiler.dart
+++ b/pkg/vm/lib/incremental_compiler.dart
@@ -262,7 +262,7 @@
// 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();
}
}
