[vm] Treat redirecting factories as ordinary factories

After https://dart-review.googlesource.com/c/sdk/+/208681
it is now possible to use bodies of redirecting factories in the VM
and treat redirecting factories like ordinary factories.

TEST=vm/cc/DartAPI_New
TEST=vm/dart/redirection_type_shuffling_test
TEST=co19/LanguageFeatures/Constructor-tear-offs

Fixes #33041
Fixes #29201
Issue #46231

Change-Id: If410d2913704a33035800144699fd6e8a2570a19
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208684
Reviewed-by: Martin Kustermann <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

pkg/front_end/testcases/strong.status

@@ -10,8 +10,6 @@ dart2js/late_statics: SemiFuzzFailure # dartbug.com/45854
 
 constructor_tearoffs/call_instantiation: TypeCheckError
 constructor_tearoffs/const_tear_off: RuntimeError
-constructor_tearoffs/redirecting_constructors: RuntimeError
-constructor_tearoffs/redirecting_factory_tear_off: RuntimeError
 extension_types/extension_on_nullable: ExpectationFileMismatchSerialized # Expected.
 extension_types/issue45775: ExpectationFileMismatchSerialized # Expected.
 extension_types/simple: ExpectationFileMismatchSerialized # Expected.

pkg/front_end/testcases/text_serialization.status

@@ -8,8 +8,6 @@
 
 constructor_tearoffs/call_instantiation: TypeCheckError
 constructor_tearoffs/const_tear_off: RuntimeError
-constructor_tearoffs/redirecting_constructors: RuntimeError
-constructor_tearoffs/redirecting_factory_tear_off: RuntimeError
 extension_types/extension_on_nullable: ExpectationFileMismatchSerialized # Expected.
 extension_types/issue45775: ExpectationFileMismatchSerialized # Expected.
 extension_types/simple: ExpectationFileMismatchSerialized # Expected.

pkg/front_end/testcases/weak.status

@@ -13,8 +13,6 @@ dart2js/late_statics: SemiFuzzFailure # dartbug.com/45854
 
 constructor_tearoffs/call_instantiation: TypeCheckError
 constructor_tearoffs/const_tear_off: RuntimeError
-constructor_tearoffs/redirecting_constructors: RuntimeError
-constructor_tearoffs/redirecting_factory_tear_off: RuntimeError
 extension_types/extension_on_nullable: ExpectationFileMismatchSerialized # Expected.
 extension_types/issue45775: ExpectationFileMismatchSerialized # Expected.
 extension_types/simple: ExpectationFileMismatchSerialized # Expected.

pkg/vm/lib/transformations/type_flow/native_code.dart

@@ -8,8 +8,6 @@ import 'dart:core' hide Type;
 
 import 'package:kernel/ast.dart';
 import 'package:kernel/library_index.dart' show LibraryIndex;
-import 'package:front_end/src/api_unstable/vm.dart'
-    show getRedirectingFactoryBody;
 
 import 'calls.dart';
 import 'types.dart';
@@ -74,26 +72,6 @@ class PragmaEntryPointsVisitor extends RecursiveVisitor {
     var type = _annotationsDefineRoot(proc.annotations);
     if (type == null) return;
 
-    if (proc.isRedirectingFactory) {
-      if (type != PragmaEntryPointType.CallOnly &&
-          type != PragmaEntryPointType.Default) {
-        throw "Error: factory $proc doesn't have a setter or getter";
-      }
-      Member target = proc;
-      while (target is Procedure && target.isRedirectingFactory) {
-        target = getRedirectingFactoryBody(target)!.target!;
-        assert(
-            (target is Procedure && target.isFactory) || target is Constructor);
-      }
-      entryPoints
-          .addRawCall(new DirectSelector(target, callKind: CallKind.Method));
-      if (target is Constructor) {
-        entryPoints.addAllocatedClass(target.enclosingClass);
-      }
-      nativeCodeOracle.setMemberReferencedFromNativeCode(target);
-      return;
-    }
-
     void addSelector(CallKind ck) {
       entryPoints.addRawCall(proc.isInstanceMember
           ? new InterfaceSelector(proc, callKind: ck)

pkg/vm/lib/transformations/type_flow/summary_collector.dart

@@ -608,7 +608,6 @@ class SummaryCollector extends RecursiveResultVisitor<TypeExpr?> {
         "${member}${fieldSummaryType == FieldSummaryType.kFieldGuard ? " (guard)" : ""}";
     debugPrint("===== $summaryName =====");
     assert(!member.isAbstract);
-    assert(!(member is Procedure && member.isRedirectingFactory));
 
     _protobufHandler?.beforeSummaryCreation(member);
 
@@ -2477,17 +2476,7 @@ class ConstantAllocationCollector extends ConstantVisitor<Type> {
     return new ConcreteType(resultClass.cls, null, constant);
   }
 
-  @override
-  Type visitStaticTearOffConstant(StaticTearOffConstant constant) {
-    final Procedure member = constant.target;
-    summaryCollector._entryPointsListener
-        .addRawCall(new DirectSelector(member));
-    summaryCollector._entryPointsListener.recordTearOff(member);
-    return _getStaticType(constant);
-  }
-
-  @override
-  Type visitConstructorTearOffConstant(ConstructorTearOffConstant constant) {
+  Type _visitTearOffConstant(TearOffConstant constant) {
     final Member member = constant.target;
     summaryCollector._entryPointsListener
         .addRawCall(new DirectSelector(member));
@@ -2499,6 +2488,19 @@ class ConstantAllocationCollector extends ConstantVisitor<Type> {
     return _getStaticType(constant);
   }
 
+  @override
+  Type visitStaticTearOffConstant(StaticTearOffConstant constant) =>
+      _visitTearOffConstant(constant);
+
+  @override
+  Type visitConstructorTearOffConstant(ConstructorTearOffConstant constant) =>
+      _visitTearOffConstant(constant);
+
+  @override
+  Type visitRedirectingFactoryTearOffConstant(
+          RedirectingFactoryTearOffConstant constant) =>
+      _visitTearOffConstant(constant);
+
   @override
   Type visitInstantiationConstant(InstantiationConstant constant) {
     constant.tearOffConstant.accept(this);

pkg/vm/lib/transformations/type_flow/transformer.dart

@@ -1943,6 +1943,12 @@ class _TreeShakerConstantVisitor extends ConstantVisitor<Null> {
     shaker.addUsedMember(constant.target);
   }
 
+  @override
+  visitRedirectingFactoryTearOffConstant(
+      RedirectingFactoryTearOffConstant constant) {
+    shaker.addUsedMember(constant.target);
+  }
+
   @override
   visitInstantiationConstant(InstantiationConstant constant) {
     analyzeConstant(constant.tearOffConstant);

pkg/vm/testcases/transformations/type_flow/transformer/regress_flutter57213.dart.expect

@@ -7,6 +7,9 @@ class A extends core::Object {
   constructor •() → self::A
     : super core::Object::•()
     ;
+  @#C3
+  static factory foo() → self::A
+    return new self::B::•();
 }
 class B extends self::A {
   constructor •() → self::B
@@ -17,11 +20,16 @@ abstract class C extends core::Object {
   constructor •() → self::C
     : super core::Object::•()
     ;
+  @#C3
+  static factory bar() → self::C
+    return [@vm.inferred-type.metadata=#lib::E] self::D::baz();
 }
 abstract class D extends self::C {
   constructor •() → self::D
     : super self::C::•()
     ;
+  static factory baz() → self::D
+    return new self::E::•();
 }
 class E extends self::D {
   constructor •() → self::E

runtime/tests/vm/dart/redirection_type_shuffling_test.dart

@@ -7,14 +7,14 @@ import "package:expect/expect.dart";
 
 class G<A extends int, B extends String> {
   G();
-  factory G.swap() = G<B,A>;  //# 00: compile-time error
-  factory G.retain() = G<A,B>;
+  factory G.swap() = G<B, A>; //# 00: compile-time error
+  factory G.retain() = G<A, B>;
 }
 
 main() {
   ClassMirror cm = reflect(new G<int, String>()).type;
 
-  Expect.isTrue(cm.newInstance(#retain, []).reflectee is G<int,String>);
+  Expect.isTrue(cm.newInstance(#retain, []).reflectee is G<int, String>);
 
-  Expect.isTrue(cm.newInstance(#swap, []).reflectee is G<String,int>);  //# 00: dynamic type error
+  Expect.isTrue(cm.newInstance(#swap, []).reflectee is G<String, int>); //# 00: continued
 }

runtime/tests/vm/dart_2/redirection_type_shuffling_test.dart

@@ -9,14 +9,14 @@ import "package:expect/expect.dart";
 
 class G<A extends int, B extends String> {
   G();
-  factory G.swap() = G<B,A>;  //# 00: compile-time error
-  factory G.retain() = G<A,B>;
+  factory G.swap() = G<B, A>; //# 00: compile-time error
+  factory G.retain() = G<A, B>;
 }
 
 main() {
   ClassMirror cm = reflect(new G<int, String>()).type;
 
-  Expect.isTrue(cm.newInstance(#retain, []).reflectee is G<int,String>);
+  Expect.isTrue(cm.newInstance(#retain, []).reflectee is G<int, String>);
 
-  Expect.isTrue(cm.newInstance(#swap, []).reflectee is G<String,int>);  //# 00: dynamic type error
+  Expect.isTrue(cm.newInstance(#swap, []).reflectee is G<String, int>); //# 00: continued
 }

runtime/tests/vm/vm.status

@@ -71,18 +71,11 @@ dart_2/regress_40753_test: Skip # This test crashes on the bot, but not locally,
 dart_2/trigger_gc_in_native_test: Skip # This test crashes on the bot, but not locally, and infrastructure repeatly fails to locate its coredump.
 
 [ $compiler == app_jitk ]
-dart/redirection_type_shuffling_test/none: RuntimeError
 dart/snapshot_version_test: RuntimeError
-dart_2/redirection_type_shuffling_test/none: RuntimeError
 dart_2/snapshot_version_test: RuntimeError
 
 [ $compiler == dartk ]
-cc/DartAPI_New: Fail # Issue #33041
 cc/IsolateReload_LibraryLookup: Fail, Crash # Issue 32190
-dart/redirection_type_shuffling_test/00: RuntimeError, Pass
-dart/redirection_type_shuffling_test/none: RuntimeError
-dart_2/redirection_type_shuffling_test/00: RuntimeError, Pass
-dart_2/redirection_type_shuffling_test/none: RuntimeError
 
 [ $compiler != dartk ]
 cc/IsolateReload_KernelIncrementalCompile: SkipByDesign
@@ -100,10 +93,12 @@ dart_2/isolates/reload_*: SkipByDesign # These tests only run on normal JIT.
 [ $compiler == dartkp ]
 dart/causal_stacks/async_throws_stack_no_causal_non_symbolic_test: SkipByDesign # --no-lazy... does nothing on precompiler.
 dart/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # --no-lazy... does nothing on precompiler.
+dart/redirection_type_shuffling_test: SkipByDesign # Uses dart:mirrors.
 dart/scavenger_abort_test: SkipSlow
 dart/v8_snapshot_profile_writer_test: Pass, Slow # Can be slow due to re-invoking the precompiler.
 dart_2/causal_stacks/async_throws_stack_no_causal_non_symbolic_test: SkipByDesign # --no-lazy... does nothing on precompiler.
 dart_2/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # --no-lazy... does nothing on precompiler.
+dart_2/redirection_type_shuffling_test: SkipByDesign # Uses dart:mirrors.
 dart_2/scavenger_abort_test: SkipSlow
 dart_2/v8_snapshot_profile_writer_test: Pass, Slow # Can be slow due to re-invoking the precompiler.
 
@@ -141,9 +136,7 @@ cc/CoreSnapshotSize: SkipByDesign # Imports dart:mirrors
 cc/CreateMirrorSystem: SkipByDesign # Imports dart:mirrors
 cc/StandaloneSnapshotSize: SkipByDesign # Imports dart:mirrors
 dart/gen_snapshot_include_resolved_urls_test: SkipByDesign # Script URLs not included in product gen_snapshot
-dart/redirection_type_shuffling_test: SkipByDesign # Imports dart:mirrors
 dart_2/gen_snapshot_include_resolved_urls_test: SkipByDesign # Script URLs not included in product gen_snapshot
-dart_2/redirection_type_shuffling_test: SkipByDesign # Imports dart:mirrors
 
 [ $nnbd == legacy ]
 dart/*: SkipByDesign # Migrated tests are not supposed to run on non-NNBD bots.
@@ -242,10 +235,6 @@ cc/IsolateReload_LibraryLookup: Fail, Crash
 [ $compiler == dartk && $system == windows ]
 cc/IsolateReload_LibraryLookup: Fail, Crash
 
-[ $compiler == dartk && $checked ]
-dart/redirection_type_shuffling_test/00: Pass # Works in --checked mode but not in --strong mode.
-dart_2/redirection_type_shuffling_test/00: Pass # Works in --checked mode but not in --strong mode.
-
 [ $compiler == dartk && ($arch == simarm || $arch == simarm64 || $arch == simarm64c) ]
 dart/appjit*: SkipSlow # DFE too slow
 dart/b162922506_test: SkipSlow # Generates large input file
@@ -287,9 +276,7 @@ dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test: Pass, Slow
 dart_2/regress_45898_test: Pass, Slow
 
 [ $compiler == dartkp && ($runtime == dart_precompiled || $runtime == vm) ]
-dart/redirection_type_shuffling_test: SkipByDesign # Includes dart:mirrors.
 dart/spawn_shutdown_test: SkipSlow
-dart_2/redirection_type_shuffling_test: SkipByDesign # Includes dart:mirrors.
 dart_2/spawn_shutdown_test: SkipSlow
 
 [ $mode == debug && $system == windows ]

runtime/vm/compiler/frontend/constant_reader.cc

@@ -345,15 +345,9 @@ InstancePtr ConstantReader::ReadConstantInternal(intptr_t constant_index) {
                               type_arguments, function, context, Heap::kOld);
       break;
     }
-    case kStaticTearOffConstant: {
-      const NameIndex index = reader.ReadCanonicalNameReference();
-      Function& function =
-          Function::Handle(Z, H.LookupStaticMethodByKernelProcedure(index));
-      function = function.ImplicitClosureFunction();
-      instance = function.ImplicitStaticClosure();
-      break;
-    }
-    case kConstructorTearOffConstant: {
+    case kStaticTearOffConstant:
+    case kConstructorTearOffConstant:
+    case kRedirectingFactoryTearOffConstant: {
       const NameIndex index = reader.ReadCanonicalNameReference();
       Function& function = Function::Handle(Z);
       if (H.IsConstructor(index)) {

runtime/vm/kernel_loader.cc

@@ -1946,8 +1946,7 @@ void KernelLoader::LoadProcedure(const Library& library,
   // additional functions can cause strain on the VM. They are therefore skipped
   // in jit mode and their associated origin function is used instead as
   // interface call target.
-  if (procedure_helper.IsRedirectingFactory() ||
-      (!FLAG_precompiled_mode && procedure_helper.IsMemberSignature())) {
+  if (!FLAG_precompiled_mode && procedure_helper.IsMemberSignature()) {
     helper_.SetOffset(procedure_end);
     return;
   }