[tfa,aot,dart2wasm] Allow tree-shaking of extension type members independently of their tear-offs

alexmarkov · Commit Queue · commit 8c1fa6d05b8d · 2024-12-18T06:10:31.000-08:00
ExtensionTypeMemberDescriptor.memberReference and ExtensionMemberDescriptor.memberReference are now nullable. TFA-based tree shaker can now set them to null when extension type member is not used and removed but corresponding lowered tear-off is still used and retained. Front-end never sets them to null and requires non-null memberReferences when consuming kernel. TEST=pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter155624.dart Fixes flutter/flutter#155624 Change-Id: I71f98c02f4659ff72a8c1d7fc6c578e8b8e26d82 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401382 Reviewed-by: Johnni Winther <johnniwinther@google.com> Reviewed-by: Slava Egorov <vegorov@google.com> Reviewed-by: Srujan Gaddam <srujzs@google.com> Reviewed-by: Ömer Ağacan <omersa@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/pkg/_js_interop_checks/lib/src/transformations/export_checker.dart b/pkg/_js_interop_checks/lib/src/transformations/export_checker.dart
@@ -250,7 +250,7 @@ extension ExtensionMemberDescriptorExtension on ExtensionMemberDescriptor {
   bool get isSetter => kind == ExtensionMemberKind.Setter;
   bool get isMethod => kind == ExtensionMemberKind.Method;
 
-  bool get isExternal => (memberReference.asProcedure).isExternal;
+  bool get isExternal => (memberReference!.asProcedure).isExternal;
 }
 
 extension ProcedureExtension on Procedure {
diff --git a/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart b/pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart
@@ -981,7 +981,7 @@ class ExtensionIndex {
     if (_processedExtensionLibraries.contains(library)) return;
     for (var extension in library.extensions) {
       for (var descriptor in extension.memberDescriptors) {
-        var reference = descriptor.memberReference;
+        var reference = descriptor.memberReference!;
         var onType = extension.onType;
         bool isInteropOnType = false;
         Annotatable? cls;
@@ -1127,7 +1127,7 @@ class ExtensionIndex {
     for (var extensionType in library.extensionTypeDeclarations) {
       if (isInteropExtensionType(extensionType)) {
         for (var descriptor in extensionType.memberDescriptors) {
-          final reference = descriptor.memberReference;
+          final reference = descriptor.memberReference!;
           _extensionTypeMemberIndex[reference] = descriptor;
           _extensionTypeIndex[reference] = extensionType;
           final tearOffReference = descriptor.tearOffReference;
diff --git a/pkg/_js_interop_checks/lib/src/transformations/shared_interop_transformer.dart b/pkg/_js_interop_checks/lib/src/transformations/shared_interop_transformer.dart
@@ -563,7 +563,7 @@ class SharedInteropTransformer extends Transformer {
         break;
       default:
         for (final descriptor in interopTypeDecl.memberDescriptors) {
-          final descriptorNode = descriptor.memberReference.node;
+          final descriptorNode = descriptor.memberReference!.node;
           if (descriptorNode is Procedure &&
               _extensionIndex.isLiteralConstructor(descriptorNode)) {
             _diagnosticReporter.report(
diff --git a/pkg/_js_interop_checks/lib/src/transformations/static_interop_mock_validator.dart b/pkg/_js_interop_checks/lib/src/transformations/static_interop_mock_validator.dart
@@ -222,7 +222,7 @@ class StaticInteropMockValidator {
   // setters return their return and parameter types, respectively.
   DartType _getTypeOfDescriptor(ExtensionMemberDescriptor interopDescriptor) {
     // CFE creates static procedures for each extension member.
-    var interopMember = interopDescriptor.memberReference.asProcedure;
+    var interopMember = interopDescriptor.memberReference!.asProcedure;
 
     if (interopDescriptor.isGetter) {
       return interopMember.function.returnType;
@@ -365,7 +365,7 @@ class StaticInteropMockValidator {
               _descriptorToExtensionName[descriptor] =
                   extension.isUnnamedExtension ? '<unnamed>' : extension.name;
               var name = js_interop.getJSName(
-                descriptor.memberReference.asMember,
+                descriptor.memberReference!.asMember,
               );
               if (name.isEmpty) name = descriptor.name.text;
               exportNameToDescriptors!
diff --git a/pkg/dart2wasm/lib/record_class_generator.dart b/pkg/dart2wasm/lib/record_class_generator.dart
@@ -139,7 +139,7 @@ class _RecordClassGenerator {
       .singleWhere((e) => e.name == 'WasmArrayExt')
       .memberDescriptors
       .singleWhere((member) => member.name.text == '[]')
-      .memberReference
+      .memberReference!
       .node as Procedure;
 
   late final Constructor wasmArrayLiteralConstructor =
diff --git a/pkg/front_end/lib/src/dill/dill_extension_builder.dart b/pkg/front_end/lib/src/dill/dill_extension_builder.dart
@@ -42,14 +42,14 @@ class DillExtensionBuilder extends ExtensionBuilderImpl
       switch (descriptor.kind) {
         case ExtensionMemberKind.Method:
           if (descriptor.isStatic) {
-            Procedure procedure = descriptor.memberReference.asProcedure;
+            Procedure procedure = descriptor.memberReference!.asProcedure;
             nameSpace.addLocalMember(
                 name.text,
                 new DillExtensionStaticMethodBuilder(
                     procedure, descriptor, libraryBuilder, this),
                 setter: false);
           } else {
-            Procedure procedure = descriptor.memberReference.asProcedure;
+            Procedure procedure = descriptor.memberReference!.asProcedure;
             Procedure? tearOff = descriptor.tearOffReference?.asProcedure;
             assert(tearOff != null, "No tear found for ${descriptor}");
             nameSpace.addLocalMember(
@@ -60,31 +60,31 @@ class DillExtensionBuilder extends ExtensionBuilderImpl
           }
           break;
         case ExtensionMemberKind.Getter:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionGetterBuilder(
                   procedure, descriptor, libraryBuilder, this),
               setter: false);
           break;
         case ExtensionMemberKind.Field:
-          Field field = descriptor.memberReference.asField;
+          Field field = descriptor.memberReference!.asField;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionFieldBuilder(
                   field, descriptor, libraryBuilder, this),
               setter: false);
           break;
         case ExtensionMemberKind.Setter:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionSetterBuilder(
                   procedure, descriptor, libraryBuilder, this),
               setter: true);
           break;
         case ExtensionMemberKind.Operator:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionOperatorBuilder(
diff --git a/pkg/front_end/lib/src/dill/dill_extension_type_declaration_builder.dart b/pkg/front_end/lib/src/dill/dill_extension_type_declaration_builder.dart
@@ -79,14 +79,14 @@ class DillExtensionTypeDeclarationBuilder
       switch (descriptor.kind) {
         case ExtensionTypeMemberKind.Method:
           if (descriptor.isStatic) {
-            Procedure procedure = descriptor.memberReference.asProcedure;
+            Procedure procedure = descriptor.memberReference!.asProcedure;
             nameSpace.addLocalMember(
                 name.text,
                 new DillExtensionTypeStaticMethodBuilder(
                     procedure, descriptor, libraryBuilder, this),
                 setter: false);
           } else {
-            Procedure procedure = descriptor.memberReference.asProcedure;
+            Procedure procedure = descriptor.memberReference!.asProcedure;
             Procedure? tearOff = descriptor.tearOffReference?.asProcedure;
             assert(tearOff != null, "No tear found for ${descriptor}");
             nameSpace.addLocalMember(
@@ -97,39 +97,39 @@ class DillExtensionTypeDeclarationBuilder
           }
           break;
         case ExtensionTypeMemberKind.Getter:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionTypeGetterBuilder(
                   procedure, descriptor, libraryBuilder, this),
               setter: false);
           break;
         case ExtensionTypeMemberKind.Field:
-          Field field = descriptor.memberReference.asField;
+          Field field = descriptor.memberReference!.asField;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionTypeFieldBuilder(
                   field, descriptor, libraryBuilder, this),
               setter: false);
           break;
         case ExtensionTypeMemberKind.Setter:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionTypeSetterBuilder(
                   procedure, descriptor, libraryBuilder, this),
               setter: true);
           break;
         case ExtensionTypeMemberKind.Operator:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           nameSpace.addLocalMember(
               name.text,
               new DillExtensionTypeOperatorBuilder(
                   procedure, descriptor, libraryBuilder, this),
               setter: false);
           break;
         case ExtensionTypeMemberKind.Constructor:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           Procedure? tearOff = descriptor.tearOffReference?.asProcedure;
           nameSpace.addConstructor(
               name.text,
@@ -138,7 +138,7 @@ class DillExtensionTypeDeclarationBuilder
           break;
         case ExtensionTypeMemberKind.Factory:
         case ExtensionTypeMemberKind.RedirectingFactory:
-          Procedure procedure = descriptor.memberReference.asProcedure;
+          Procedure procedure = descriptor.memberReference!.asProcedure;
           Procedure? tearOff = descriptor.tearOffReference?.asProcedure;
           nameSpace.addConstructor(
               name.text,
diff --git a/pkg/front_end/lib/src/kernel/exhaustiveness.dart b/pkg/front_end/lib/src/kernel/exhaustiveness.dart
@@ -199,7 +199,7 @@ class CfeTypeOperations implements TypeOperations<DartType> {
             fieldTypes[new NameKey(descriptor.name.text)] =
                 functionType.returnType;
           case ExtensionTypeMemberKind.Getter:
-            Procedure member = descriptor.memberReference.asProcedure;
+            Procedure member = descriptor.memberReference!.asProcedure;
             FunctionType functionType = member.getterType as FunctionType;
             if (extensionTypeDeclaration.typeParameters.isNotEmpty) {
               functionType = FunctionTypeInstantiator.instantiate(
diff --git a/pkg/front_end/lib/src/testing/id_testing_utils.dart b/pkg/front_end/lib/src/testing/id_testing_utils.dart
@@ -911,7 +911,8 @@ List<String> extensionMethodDescriptorToText(
   }
 
   return [
-    descriptorToText(descriptor.memberReference, forTearOff: false),
+    if (descriptor.memberReference != null)
+      descriptorToText(descriptor.memberReference!, forTearOff: false),
     if (descriptor.tearOffReference != null)
       descriptorToText(descriptor.tearOffReference!, forTearOff: true),
   ];
diff --git a/pkg/kernel/binary.md b/pkg/kernel/binary.md
@@ -147,7 +147,7 @@ type CanonicalName {
 
 type ComponentFile {
   UInt32 magic = 0x90ABCDEF;
-  UInt32 formatVersion = 121;
+  UInt32 formatVersion = 122;
   Byte[10] shortSdkHash;
   List<String> problemsAsJson; // Described in problems.md.
   Library[] libraries;
@@ -358,8 +358,8 @@ type ExtensionMemberDescriptor {
   Name name;
   ExtensionMemberKind kind;
   Byte flags (isStatic);
-  MemberReference member;
-  MemberReference tearOff;
+  MemberReference member;  // May be NullReference.
+  MemberReference tearOff; // May be NullReference.
 }
 
 type ExtensionTypeDeclaration extends Node {
@@ -384,8 +384,8 @@ type ExtensionTypeMemberDescriptor {
   Name name;
   ExtensionTypeMemberKind kind;
   Byte flags (isStatic);
-  MemberReference member;
-  MemberReference tearOff;
+  MemberReference member;  // May be NullReference.
+  MemberReference tearOff; // May be NullReference.
 }
 
 abstract type Member extends Node {}
diff --git a/pkg/kernel/lib/binary/ast_from_binary.dart b/pkg/kernel/lib/binary/ast_from_binary.dart
@@ -1600,12 +1600,12 @@ class BinaryBuilder {
     Name name = readName();
     int kind = readByte();
     int flags = readByte();
-    CanonicalName memberName = readNonNullCanonicalNameReference();
+    CanonicalName? memberName = readNullableCanonicalNameReference();
     CanonicalName? tearOffName = readNullableCanonicalNameReference();
     return new ExtensionMemberDescriptor(
         name: name,
         kind: ExtensionMemberKind.values[kind],
-        memberReference: memberName.reference,
+        memberReference: memberName?.reference,
         tearOffReference: tearOffName?.reference)
       ..flags = flags;
   }
@@ -1692,12 +1692,12 @@ class BinaryBuilder {
     Name name = readName();
     int kind = readByte();
     int flags = readByte();
-    CanonicalName memberName = readNonNullCanonicalNameReference();
+    CanonicalName? memberName = readNullableCanonicalNameReference();
     CanonicalName? tearOffName = readNullableCanonicalNameReference();
     return new ExtensionTypeMemberDescriptor(
         name: name,
         kind: ExtensionTypeMemberKind.values[kind],
-        memberReference: memberName.reference,
+        memberReference: memberName?.reference,
         tearOffReference: tearOffName?.reference)
       ..flags = flags;
   }
diff --git a/pkg/kernel/lib/binary/ast_to_binary.dart b/pkg/kernel/lib/binary/ast_to_binary.dart
@@ -2648,10 +2648,14 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
       writeName(descriptor.name);
       writeByte(descriptor.kind.index);
       writeByte(descriptor.flags);
-      assert(descriptor.memberReference.canonicalName != null,
+      final Reference? memberReference = descriptor.memberReference;
+      assert(memberReference == null || memberReference.canonicalName != null,
           "No canonical name for ${descriptor}.");
-      writeNonNullCanonicalNameReference(descriptor.memberReference);
-      writeNullAllowedCanonicalNameReference(descriptor.tearOffReference);
+      writeNullAllowedCanonicalNameReference(memberReference);
+      final Reference? tearOffReference = descriptor.tearOffReference;
+      assert(tearOffReference == null || tearOffReference.canonicalName != null,
+          "No canonical name for ${descriptor} tear-off.");
+      writeNullAllowedCanonicalNameReference(tearOffReference);
     }
   }
 
@@ -2689,14 +2693,14 @@ class BinaryPrinter implements Visitor<void>, BinarySink {
       writeName(descriptor.name);
       writeByte(descriptor.kind.index);
       writeByte(descriptor.flags);
-      assert(descriptor.memberReference.canonicalName != null,
+      final Reference? memberReference = descriptor.memberReference;
+      assert(memberReference == null || memberReference.canonicalName != null,
           "No canonical name for ${descriptor}.");
-      writeNonNullCanonicalNameReference(descriptor.memberReference);
-      assert(
-          descriptor.tearOffReference == null ||
-              descriptor.tearOffReference?.canonicalName != null,
+      writeNullAllowedCanonicalNameReference(memberReference);
+      final Reference? tearOffReference = descriptor.tearOffReference;
+      assert(tearOffReference == null || tearOffReference.canonicalName != null,
           "No canonical name for ${descriptor} tear-off.");
-      writeNullAllowedCanonicalNameReference(descriptor.tearOffReference);
+      writeNullAllowedCanonicalNameReference(tearOffReference);
     }
   }
 
diff --git a/pkg/kernel/lib/binary/tag.dart b/pkg/kernel/lib/binary/tag.dart
@@ -226,7 +226,7 @@ class Tag {
   /// Internal version of kernel binary format.
   /// Bump it when making incompatible changes in kernel binaries.
   /// Keep in sync with runtime/vm/kernel_binary.h, pkg/kernel/binary.md.
-  static const int BinaryFormatVersion = 121;
+  static const int BinaryFormatVersion = 122;
 }
 
 abstract class ConstantTag {
diff --git a/pkg/kernel/lib/src/ast/declarations.dart b/pkg/kernel/lib/src/ast/declarations.dart
@@ -748,7 +748,9 @@ class ExtensionMemberDescriptor {
   int flags = 0;
 
   /// Reference to the top-level member created for the extension method.
-  final Reference memberReference;
+  /// This member reference is not null after the front-end but can
+  /// be cleared by certain back-ends (e.g. VM/AOT) if member is not used.
+  final Reference? memberReference;
 
   /// Reference to the top-level member created for the extension member tear
   /// off, if any.
@@ -761,6 +763,7 @@ class ExtensionMemberDescriptor {
       required this.memberReference,
       required this.tearOffReference}) {
     this.isStatic = isStatic;
+    assert(memberReference != null || tearOffReference != null);
   }
 
   /// Return `true` if the extension method was declared as `static`.
@@ -773,7 +776,7 @@ class ExtensionMemberDescriptor {
   @override
   String toString() {
     return 'ExtensionMemberDescriptor($name,$kind,'
-        '${memberReference.toStringInternal()},isStatic=${isStatic})';
+        '${memberReference?.toStringInternal()},isStatic=${isStatic})';
   }
 }
 
@@ -1034,7 +1037,9 @@ class ExtensionTypeMemberDescriptor {
 
   /// Reference to the top-level member created for the extension type
   /// declaration member.
-  final Reference memberReference;
+  /// This member reference is not null after the front-end but can
+  /// be cleared by certain back-ends (e.g. VM/AOT) if member is not used.
+  final Reference? memberReference;
 
   /// Reference to the top-level member created for the extension type
   /// declaration member tear off, if any.
@@ -1047,6 +1052,7 @@ class ExtensionTypeMemberDescriptor {
       required this.memberReference,
       required this.tearOffReference}) {
     this.isStatic = isStatic;
+    assert(memberReference != null || tearOffReference != null);
   }
 
   /// Return `true` if the extension type declaration member was declared as
@@ -1060,7 +1066,7 @@ class ExtensionTypeMemberDescriptor {
   @override
   String toString() {
     return 'ExtensionTypeMemberDescriptor($name,$kind,'
-        '${memberReference.toStringInternal()},isStatic=${isStatic},'
+        '${memberReference?.toStringInternal()},isStatic=${isStatic},'
         '${tearOffReference?.toStringInternal()})';
   }
 }
diff --git a/pkg/kernel/lib/text/ast_to_text.dart b/pkg/kernel/lib/text/ast_to_text.dart
diff --git a/pkg/kernel/lib/transformations/track_widget_constructor_locations.dart b/pkg/kernel/lib/transformations/track_widget_constructor_locations.dart
diff --git a/pkg/kernel/lib/verifier.dart b/pkg/kernel/lib/verifier.dart
diff --git a/pkg/vm/lib/transformations/type_flow/transformer.dart b/pkg/vm/lib/transformations/type_flow/transformer.dart
diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter155624.dart b/pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter155624.dart
diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter155624.dart.expect b/pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter155624.dart.expect
diff --git a/runtime/vm/kernel_binary.h b/runtime/vm/kernel_binary.h

Original file line number	Diff line number	Diff line change
`@@ -250,7 +250,7 @@ extension ExtensionMemberDescriptorExtension on ExtensionMemberDescriptor {`
`250`	`250`	`bool get isSetter => kind == ExtensionMemberKind.Setter;`
`251`	`251`	`bool get isMethod => kind == ExtensionMemberKind.Method;`
`252`	`252`
`253`		`- bool get isExternal => (memberReference.asProcedure).isExternal;`
	`253`	`+ bool get isExternal => (memberReference!.asProcedure).isExternal;`
`254`	`254`	`}`
`255`	`255`
`256`	`256`	`extension ProcedureExtension on Procedure {`
Original file line number	Diff line number	Diff line change
`@@ -911,7 +911,8 @@ List<String> extensionMethodDescriptorToText(`
`911`	`911`	`}`
`912`	`912`
`913`	`913`	`return [`
`914`		`- descriptorToText(descriptor.memberReference, forTearOff: false),`
	`914`	`+ if (descriptor.memberReference != null)`
	`915`	`+ descriptorToText(descriptor.memberReference!, forTearOff: false),`
`915`	`916`	`if (descriptor.tearOffReference != null)`
`916`	`917`	`descriptorToText(descriptor.tearOffReference!, forTearOff: true),`
`917`	`918`	`];`