[VM] [CFE]: Expression compilation inside extension method

This CL handles expression compilation inside extension methods better.
It is now possible to evaluate "this" and other methods defined in the
extension.

#46757

TEST=service tests added.

Change-Id: I3c71eb23117e26b01961f32103f4046f0b537976
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212286
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Ben Konyi <[email protected]>
Reviewed-by: Anna Gringauze <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: jensjoha

.dart_tool/package_config.json

@@ -11,7 +11,7 @@
     "constraint, update this by running tools/generate_package_config.dart."
   ],
   "configVersion": 2,
-  "generated": "2021-09-08T14:55:24.803714",
+  "generated": "2021-09-10T10:06:41.987732",
   "generator": "tools/generate_package_config.dart",
   "packages": [
     {
@@ -494,7 +494,7 @@
       "name": "observatory_2",
       "rootUri": "../runtime/observatory_2",
       "packageUri": "lib/",
-      "languageVersion": "2.2"
+      "languageVersion": "2.6"
     },
     {
       "name": "observatory_test_package",

pkg/dev_compiler/lib/src/kernel/expression_compiler.dart

@@ -440,8 +440,8 @@ class ExpressionCompiler {
         scope.typeParameters,
         debugProcedureName,
         scope.library.importUri,
-        scope.cls?.name,
-        scope.isStatic);
+        className: scope.cls?.name,
+        isStatic: scope.isStatic);
 
     _log('Compiled expression to kernel');
 

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

@@ -136,8 +136,9 @@ abstract class IncrementalKernelGenerator {
       List<TypeParameter> typeDefinitions,
       String syntheticProcedureName,
       Uri libraryUri,
-      [String? className,
-      bool isStatic = false]);
+      {String? className,
+      String? methodName,
+      bool isStatic = false});
 
   /// Sets experimental features.
   ///

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

@@ -660,8 +660,8 @@ String extractLocalNameFromLateLoweredSetter(String name) {
 ///
 /// where '#this' is the synthetic "extension this" parameter.
 bool isExtensionThis(VariableDeclaration node) {
-  assert(
-      node.isLowered || node.name == null || !isExtensionThisName(node.name));
+  assert(node.isLowered || node.name == null || !isExtensionThisName(node.name),
+      "$node has name ${node.name} and node.isLowered = ${node.isLowered}");
   return node.isLowered && isExtensionThisName(node.name);
 }
 

pkg/front_end/lib/src/api_unstable/vm.dart

@@ -29,6 +29,8 @@ export '../api_prototype/incremental_kernel_generator.dart'
 export '../api_prototype/kernel_generator.dart'
     show kernelForModule, kernelForProgram;
 
+export '../api_prototype/lowering_predicates.dart' show isExtensionThisName;
+
 export '../api_prototype/memory_file_system.dart' show MemoryFileSystem;
 
 export '../api_prototype/standard_file_system.dart' show StandardFileSystem;

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

@@ -11,7 +11,15 @@ import 'package:_fe_analyzer_shared/src/scanner/abstract_scanner.dart'
 
 import 'package:front_end/src/api_prototype/experimental_flags.dart';
 import 'package:front_end/src/api_prototype/front_end.dart';
+
+import 'package:front_end/src/api_prototype/lowering_predicates.dart'
+    show isExtensionThisName;
+
 import 'package:front_end/src/base/nnbd_mode.dart';
+
+import 'package:front_end/src/fasta/builder/member_builder.dart'
+    show MemberBuilder;
+
 import 'package:front_end/src/fasta/fasta_codes.dart';
 import 'package:front_end/src/fasta/source/source_loader.dart';
 import 'package:kernel/binary/ast_from_binary.dart'
@@ -34,6 +42,7 @@ import 'package:kernel/kernel.dart'
         Component,
         DartType,
         Expression,
+        Extension,
         FunctionNode,
         Library,
         LibraryDependency,
@@ -48,7 +57,8 @@ import 'package:kernel/kernel.dart'
         Source,
         Supertype,
         TreeNode,
-        TypeParameter;
+        TypeParameter,
+        VariableDeclaration;
 
 import 'package:kernel/canonical_name.dart'
     show CanonicalNameError, CanonicalNameSdkError;
@@ -71,6 +81,8 @@ import 'builder/builder.dart' show Builder;
 
 import 'builder/class_builder.dart' show ClassBuilder;
 
+import 'builder/extension_builder.dart' show ExtensionBuilder;
+
 import 'builder/field_builder.dart' show FieldBuilder;
 
 import 'builder/library_builder.dart' show LibraryBuilder;
@@ -1907,8 +1919,9 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       List<TypeParameter> typeDefinitions,
       String syntheticProcedureName,
       Uri libraryUri,
-      [String? className,
-      bool isStatic = false]) async {
+      {String? className,
+      String? methodName,
+      bool isStatic = false}) async {
     assert(dillLoadedData != null && userCode != null);
 
     return await context.runInContext((_) async {
@@ -1923,6 +1936,26 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         cls = classBuilder?.cls;
         if (cls == null) return null;
       }
+      Extension? extension;
+      String? extensionName;
+      if (methodName != null) {
+        int indexOfDot = methodName.indexOf(".");
+        if (indexOfDot >= 0) {
+          String beforeDot = methodName.substring(0, indexOfDot);
+          String afterDot = methodName.substring(indexOfDot + 1);
+          Builder? builder = libraryBuilder.scopeBuilder[beforeDot];
+          extensionName = beforeDot;
+          if (builder is ExtensionBuilder) {
+            extension = builder.extension;
+            Builder? subBuilder = builder.scopeBuilder[afterDot];
+            if (subBuilder is MemberBuilder) {
+              if (subBuilder.isExtensionInstanceMember) {
+                isStatic = false;
+              }
+            }
+          }
+        }
+      }
 
       userCode!.loader.resetSeenMessages();
 
@@ -1937,8 +1970,14 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           return null;
         }
       }
+      int index = 0;
       for (String name in definitions.keys) {
-        if (!isLegalIdentifier(name)) {
+        index++;
+        if (!(isLegalIdentifier(name) ||
+            (extension != null &&
+                !isStatic &&
+                index == 1 &&
+                isExtensionThisName(name)))) {
           userCode!.loader.addProblem(
               templateIncrementalCompilerIllegalParameter.withArguments(name),
               // TODO: pass variable declarations instead of
@@ -2009,13 +2048,29 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
           positionalParameters: definitions.keys
               .map((name) =>
                   new VariableDeclarationImpl(name, 0, type: definitions[name])
-                    ..fileOffset =
-                        cls?.fileOffset ?? libraryBuilder.library.fileOffset)
+                    ..fileOffset = cls?.fileOffset ??
+                        extension?.fileOffset ??
+                        libraryBuilder.library.fileOffset)
               .toList());
 
+      VariableDeclaration? extensionThis;
+      if (extension != 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;
+        }
+      }
+
       debugLibrary.build(userCode!.loader.coreLibrary, modifyTarget: false);
       Expression compiledExpression = await userCode!.loader.buildExpression(
-          debugLibrary, className, className != null && !isStatic, parameters);
+          debugLibrary,
+          className ?? extensionName,
+          (className != null && !isStatic) || extensionThis != null,
+          parameters,
+          extensionThis);
 
       Procedure procedure = new Procedure(
           new Name(syntheticProcedureName), ProcedureKind.Method, parameters,
@@ -2026,7 +2081,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         ..parent = parameters;
 
       procedure.fileUri = debugLibrary.fileUri;
-      procedure.parent = className != null ? cls : libraryBuilder.library;
+      procedure.parent = cls ?? libraryBuilder.library;
 
       userCode!.uriToSource.remove(debugExprUri);
       userCode!.loader.sourceBytes.remove(debugExprUri);

pkg/front_end/lib/src/fasta/source/source_loader.dart

@@ -45,6 +45,7 @@ import 'package:kernel/ast.dart'
         Reference,
         Supertype,
         TreeNode,
+        VariableDeclaration,
         Version;
 
 import 'package:kernel/class_hierarchy.dart'
@@ -513,26 +514,32 @@ class SourceLoader extends Loader {
     }
   }
 
-  // TODO(johnniwinther,jensj): Handle expression in extensions?
   Future<Expression> buildExpression(
       SourceLibraryBuilder libraryBuilder,
-      String? enclosingClass,
+      String? enclosingClassOrExtension,
       bool isClassInstanceMember,
-      FunctionNode parameters) async {
+      FunctionNode parameters,
+      VariableDeclaration? extensionThis) async {
     Token token = await tokenize(libraryBuilder, suppressLexicalErrors: false);
     DietListener dietListener = createDietListener(libraryBuilder);
 
     Builder parent = libraryBuilder;
-    if (enclosingClass != null) {
+    if (enclosingClassOrExtension != null) {
       Builder? cls = dietListener.memberScope
-          .lookup(enclosingClass, -1, libraryBuilder.fileUri);
+          .lookup(enclosingClassOrExtension, -1, libraryBuilder.fileUri);
       if (cls is ClassBuilder) {
         parent = cls;
         dietListener
           ..currentDeclaration = cls
           ..memberScope = cls.scope.copyWithParent(
               dietListener.memberScope.withTypeVariables(cls.typeVariables),
-              "debugExpression in $enclosingClass");
+              "debugExpression in class $enclosingClassOrExtension");
+      } else if (cls is ExtensionBuilder) {
+        parent = cls;
+        dietListener
+          ..currentDeclaration = cls
+          ..memberScope = cls.scope.copyWithParent(dietListener.memberScope,
+              "debugExpression in extension $enclosingClassOrExtension");
       }
     }
     ProcedureBuilder builder = new SourceProcedureBuilder(
@@ -562,7 +569,8 @@ class SourceLoader extends Loader {
       ..parent = parent;
     BodyBuilder listener = dietListener.createListener(
         builder, dietListener.memberScope,
-        isDeclarationInstanceMember: isClassInstanceMember);
+        isDeclarationInstanceMember: isClassInstanceMember,
+        extensionThis: extensionThis);
 
     return listener.parseSingleExpression(
         new Parser(listener,

pkg/front_end/test/fasta/expression_suite.dart

@@ -140,6 +140,8 @@ class TestCase {
 
   final String className;
 
+  final String methodName;
+
   String expression;
 
   List<CompilationResult> results = [];
@@ -153,6 +155,7 @@ class TestCase {
       this.isStaticMethod,
       this.library,
       this.className,
+      this.methodName,
       this.expression);
 
   @override
@@ -260,6 +263,7 @@ class ReadTest extends Step<TestDescription, List<TestCase>, Context> {
     bool isStaticMethod = false;
     Uri library;
     String className;
+    String methodName;
     String expression;
 
     dynamic maps = loadYamlNode(contents, sourceUrl: uri);
@@ -281,6 +285,8 @@ class ReadTest extends Step<TestDescription, List<TestCase>, Context> {
           if (uri.fragment != null && uri.fragment != '') {
             className = uri.fragment;
           }
+        } else if (key == "method") {
+          methodName = value as String;
         } else if (key == "definitions") {
           definitions = (value as YamlList).map((x) => x as String).toList();
         } else if (key == "type_definitions") {
@@ -292,8 +298,17 @@ class ReadTest extends Step<TestDescription, List<TestCase>, Context> {
           expression = value;
         }
       }
-      var test = new TestCase(description, entryPoint, import, definitions,
-          typeDefinitions, isStaticMethod, library, className, expression);
+      var test = new TestCase(
+          description,
+          entryPoint,
+          import,
+          definitions,
+          typeDefinitions,
+          isStaticMethod,
+          library,
+          className,
+          methodName,
+          expression);
       var result = test.validate();
       if (result != null) {
         return new Result.fail(tests, result);
@@ -325,13 +340,15 @@ class CompileExpression extends Step<List<TestCase>, List<TestCase>, Context> {
     }
 
     Procedure compiledProcedure = await compiler.compileExpression(
-        test.expression,
-        definitions,
-        typeParams,
-        "debugExpr",
-        test.library,
-        test.className,
-        test.isStaticMethod);
+      test.expression,
+      definitions,
+      typeParams,
+      "debugExpr",
+      test.library,
+      className: test.className,
+      methodName: test.methodName,
+      isStatic: test.isStaticMethod,
+    );
     List<DiagnosticMessage> errors = context.takeErrors();
     test.results.add(new CompilationResult(compiledProcedure, errors));
     if (compiledProcedure != null) {

pkg/front_end/testcases/expression/extension_this.expression.yaml

@@ -0,0 +1,10 @@
+# Copyright (c) 2021, 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.
+
+entry_point: "main.dart"
+definitions: ["#this"]
+position: "main.dart"
+method: "Foo.parseAsInt"
+expression: |
+  () { print(getFortyTwo()); return this; }

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

@@ -0,0 +1,7 @@
+Errors: {
+}
+method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr(lowered dynamic #this) → dynamic
+  return () → dynamic {
+    dart.core::print(main::Foo|getFortyTwo(#this as{TypeError,ForDynamic} dart.core::String*));
+    return #this;
+  };