[vm,dynamic_modules] Support members which are overridden implictly (…

…transitively) by a dynamic module Consider the following situation: member M1 is overridden by another member M2; M2 is overridden in a dynamic module. Members which can be overridden in a dynamic module (such as M2) should be specified as 'can-be-overridden' in the dynamic interface. Members which are overridden implicitly/transitively (such as M1) are not required to be mentioned in the dynamic interface. However, when determining possible targets for a call with interface target M1, compiler should treat it as potentially overridden in a dynamic module. This change adds such handling to the VM/AOT. Dynamic interface annotator now marks members such as M1 with 'dyn-module:can-be-overridden-implicitly' pragma, and VM/AOT takes both can-be-overridden and can-be-overridden-implicitly into account. This change also simplifies handling of implicitly extenable classes in the VM/AOT - now VM handles both extendable and implicitly-extendable pragmas (from dynamic interface annotator) instead of recalculating implicitly extendable classes on its own. TEST=pkg/vm/test/transformations/dynamic_interface_annotator_test.dart TEST=dynamic_modules_suite/implicitly_extendable Fixes #59880 Change-Id: Id4570cc86303f8e45a061d696e9bca0d0b2b4b81 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403951 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Nate Biggs <[email protected]>
dart-lang · Jan 15, 2025 · 9e8e8e5 · 9e8e8e5
1 parent 493de63
commit 9e8e8e5
Show file tree

Hide file tree

Showing 12 changed files with 148 additions and 71 deletions.
diff --git a/pkg/vm/lib/transformations/dynamic_interface_annotator.dart b/pkg/vm/lib/transformations/dynamic_interface_annotator.dart
@@ -5,6 +5,7 @@
 import 'package:front_end/src/api_prototype/dynamic_module_validator.dart'
     show DynamicInterfaceSpecification;
 import 'package:kernel/ast.dart';
+import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
 import 'package:kernel/core_types.dart' show CoreTypes;
 
 import 'pragma.dart'
@@ -13,7 +14,8 @@ import 'pragma.dart'
         kDynModuleCallablePragmaName,
         kDynModuleExtendablePragmaName,
         kDynModuleImplicitlyCallablePragmaName,
-        kDynModuleImplicitlyExtendablePragmaName;
+        kDynModuleImplicitlyExtendablePragmaName,
+        kDynModuleCanBeOverriddenImplicitlyPragmaName;
 
 const bool _debug = false;
 
@@ -33,13 +35,28 @@ void annotateComponent(String dynamicInterfaceSpecification, Uri baseUri,
       annotateFinalClasses: false,
       annotateStaticMembers: false,
       annotateInstanceMembers: false);
-  annotateImplicitlyExtendable(coreTypes, extendableAnnotator.annotatedClasses);
-  annotateNodes(spec.canBeOverridden, kDynModuleCanBeOverriddenPragmaName,
-      baseUri, coreTypes,
+
+  _ImplicitExtendableAnnotator(
+          pragmaConstant(coreTypes, kDynModuleImplicitlyExtendablePragmaName),
+          extendableAnnotator.annotatedClasses)
+      .annotate();
+
+  final canBeOverriddenAnnotator = annotateNodes(spec.canBeOverridden,
+      kDynModuleCanBeOverriddenPragmaName, baseUri, coreTypes,
       annotateClasses: false,
       annotateFinalClasses: true,
       annotateStaticMembers: false,
       annotateInstanceMembers: true);
+
+  final hierarchy = ClassHierarchy(component, coreTypes);
+  pragmaConstant(coreTypes, kDynModuleCanBeOverriddenImplicitlyPragmaName);
+  _ImplicitOverridesAnnotator(
+          pragmaConstant(
+              coreTypes, kDynModuleCanBeOverriddenImplicitlyPragmaName),
+          hierarchy,
+          canBeOverriddenAnnotator.annotatedMembers)
+      .annotate();
+
   final callableAnnotator = annotateNodes(
       spec.callable, kDynModuleCallablePragmaName, baseUri, coreTypes,
       annotateClasses: true,
@@ -171,16 +188,73 @@ class _Annotator extends RecursiveVisitor {
   }
 }
 
-void annotateImplicitlyExtendable(
-    CoreTypes coreTypes, Set<Class> extendableClasses) {
-  final pragma =
-      pragmaConstant(coreTypes, kDynModuleImplicitlyExtendablePragmaName);
-  for (final cls in [...extendableClasses]) {
+class _ImplicitExtendableAnnotator {
+  final Constant pragma;
+  final Set<Class> extendableClasses;
+  final Set<Class> implicitlyExtendable = Set<Class>.identity();
+
+  _ImplicitExtendableAnnotator(this.pragma, this.extendableClasses);
+
+  void annotate() {
+    for (final cls in extendableClasses) {
+      annotateSupertypesOf(cls);
+    }
+  }
+
+  void annotateSupertypesOf(Class cls) {
     for (final supertype in cls.supers) {
       final supertypeClass = supertype.classNode;
-      if (extendableClasses.add(supertypeClass)) {
+      if (implicitlyExtendable.add(supertypeClass) &&
+          !extendableClasses.contains(supertypeClass)) {
         supertypeClass.addAnnotation(ConstantExpression(pragma));
+        annotateSupertypesOf(supertypeClass);
+      }
+    }
+  }
+}
+
+class _ImplicitOverridesAnnotator {
+  final Constant pragma;
+  final ClassHierarchy hierarchy;
+  final Set<Member> overriddenMembers;
+  final Set<Member> implicitlyOverriddenSetters = Set<Member>.identity();
+  final Set<Member> implicitlyOverriddenNonSetters = Set<Member>.identity();
+
+  _ImplicitOverridesAnnotator(
+      this.pragma, this.hierarchy, this.overriddenMembers);
+
+  void annotate() {
+    for (final member in overriddenMembers) {
+      final cls = member.enclosingClass!;
+      if (member.hasGetter) {
+        annotateSupertypesOf(cls, member.name, setter: false);
+      }
+      if (member.hasSetter) {
+        annotateSupertypesOf(cls, member.name, setter: true);
+      }
+    }
+  }
+
+  void annotateSupertypesOf(Class cls, Name memberName,
+      {required bool setter}) {
+    for (final supertype in cls.supers) {
+      final supertypeClass = supertype.classNode;
+      final member = ClassHierarchy.findMemberByName(
+          hierarchy.getDeclaredMembers(supertypeClass, setters: setter),
+          memberName);
+      if (member != null) {
+        final implicitlyOverridden = setter
+            ? implicitlyOverriddenSetters
+            : implicitlyOverriddenNonSetters;
+        if (implicitlyOverridden.add(member) &&
+            !overriddenMembers.contains(member)) {
+          member.addAnnotation(ConstantExpression(pragma));
+        } else {
+          // The member is already annotated - do not go deeper.
+          continue;
+        }
       }
+      annotateSupertypesOf(supertypeClass, memberName, setter: setter);
     }
   }
 }

diff --git a/pkg/vm/lib/transformations/pragma.dart b/pkg/vm/lib/transformations/pragma.dart
@@ -26,6 +26,8 @@ const kDynModuleExtendablePragmaName = "dyn-module:extendable";
 const kDynModuleImplicitlyExtendablePragmaName =
     "dyn-module:implicitly-extendable";
 const kDynModuleCanBeOverriddenPragmaName = "dyn-module:can-be-overridden";
+const kDynModuleCanBeOverriddenImplicitlyPragmaName =
+    "dyn-module:can-be-overridden-implicitly";
 const kDynModuleCallablePragmaName = "dyn-module:callable";
 const kDynModuleImplicitlyCallablePragmaName = "dyn-module:implicitly-callable";
 const kDynModuleEntryPointPragmaName = "dyn-module:entry-point";

diff --git a/pkg/vm/testcases/transformations/dynamic_interface_annotator/dynamic_interface.yaml b/pkg/vm/testcases/transformations/dynamic_interface_annotator/dynamic_interface.yaml
@@ -18,7 +18,7 @@ extendable:
 
 can-be-overridden:
   - library: 'lib1.dart'
-    class: ['A', 'U']
+    class: ['A', 'Q', 'U']
   - library: 'lib2.dart'
     class: 'D'
     member: 'build'
diff --git a/pkg/vm/testcases/transformations/dynamic_interface_annotator/lib1.dart b/pkg/vm/testcases/transformations/dynamic_interface_annotator/lib1.dart
@@ -79,9 +79,17 @@ class N {}
 
 class O extends N {}
 
-class P {}
+class P1 implements P2 {
+  void foo() {}
+}
 
-class Q implements P {}
+abstract class P2 {
+  void foo();
+}
+
+class Q implements P1 {
+  void foo() {}
+}
 
 final class R {}
 

diff --git a/pkg/vm/testcases/transformations/dynamic_interface_annotator/lib1.dart.expect b/pkg/vm/testcases/transformations/dynamic_interface_annotator/lib1.dart.expect
@@ -141,19 +141,36 @@ class O extends self::N {
 }
 @#C11
 @#C5
-class P extends core::Object {
+class P1 extends core::Object implements self::P2 {
   @#C5
-  synthetic constructor •() → self::P
+  synthetic constructor •() → self::P1
     : super core::Object::•()
     ;
+  @#C13
+  @#C5
+  method foo() → void {}
+}
+@#C11
+@#C5
+abstract class P2 extends core::Object {
+  @#C5
+  synthetic constructor •() → self::P2
+    : super core::Object::•()
+    ;
+  @#C13
+  @#C5
+  abstract method foo() → void;
 }
 @#C3
 @#C5
-class Q extends core::Object implements self::P {
+class Q extends core::Object implements self::P1 {
   @#C5
   synthetic constructor •() → self::Q
     : super core::Object::•()
     ;
+  @#C7
+  @#C5
+  method foo() → void {}
 }
 @#C5
 final class R extends core::Object {
@@ -199,7 +216,7 @@ base class V extends core::Object {
 static field core::Object? sfield9;
 static field core::Object? _sfield10;
 @#C5
-static const field core::List<core::Object> const11 = #C16;
+static const field core::List<core::Object> const11 = #C18;
 @#C5
 static method smethod10() → void {}
 static method _smethod11() → void {}
@@ -215,9 +232,11 @@ constants  {
   #C9 = core::pragma {name:#C8, options:#C2}
   #C10 = "dyn-module:implicitly-extendable"
   #C11 = core::pragma {name:#C10, options:#C2}
-  #C12 = 2
-  #C13 = 1
-  #C14 = self::_E2 {_y:#C12, _x:#C13}
-  #C15 = redirecting-factory-tearoff self::_F::_foo
-  #C16 = <core::Object>[#C14, #C15]
+  #C12 = "dyn-module:can-be-overridden-implicitly"
+  #C13 = core::pragma {name:#C12, options:#C2}
+  #C14 = 2
+  #C15 = 1
+  #C16 = self::_E2 {_y:#C14, _x:#C15}
+  #C17 = redirecting-factory-tearoff self::_F::_foo
+  #C18 = <core::Object>[#C16, #C17]
 }
diff --git a/runtime/tests/vm/dart/dynamic_module_pragmas_il_test.dart b/runtime/tests/vm/dart/dynamic_module_pragmas_il_test.dart
@@ -55,8 +55,10 @@ void testIsA1(obj) {
   myprint(obj is C1);
 }
 
+@pragma('dyn-module:implicitly-extendable')
 abstract class A2 {
   void foo();
+  @pragma('dyn-module:can-be-overridden-implicitly')
   void bar();
   void baz();
 }

diff --git a/runtime/vm/class_finalizer.cc b/runtime/vm/class_finalizer.cc
@@ -589,10 +589,6 @@ void ClassFinalizer::FinalizeTypesInClass(const Class& cls) {
   if (is_future_subtype && !cls.is_abstract()) {
     MarkClassCanBeFuture(zone, cls);
   }
-  if (cls.is_dynamically_extendable()) {
-    MarkClassHasDynamicallyExtendableSubtypes(zone, cls);
-  }
-
   ClassHiearchyUpdater(zone).Register(cls);
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
@@ -687,25 +683,6 @@ void ClassFinalizer::MarkClassCanBeFuture(Zone* zone, const Class& cls) {
   }
 }
 
-void ClassFinalizer::MarkClassHasDynamicallyExtendableSubtypes(
-    Zone* zone,
-    const Class& cls) {
-  if (cls.has_dynamically_extendable_subtypes()) return;
-
-  cls.set_has_dynamically_extendable_subtypes(true);
-
-  Class& base = Class::Handle(zone, cls.SuperClass());
-  if (!base.IsNull()) {
-    MarkClassHasDynamicallyExtendableSubtypes(zone, base);
-  }
-  auto& interfaces = Array::Handle(zone, cls.interfaces());
-  auto& type = AbstractType::Handle(zone);
-  for (intptr_t i = 0; i < interfaces.Length(); ++i) {
-    type ^= interfaces.At(i);
-    base = type.type_class();
-    MarkClassHasDynamicallyExtendableSubtypes(zone, base);
-  }
-}
 #endif  // defined(DART_PRECOMPILED_RUNTIME)
 
 void ClassFinalizer::FinalizeClass(const Class& cls) {

diff --git a/runtime/vm/class_finalizer.h b/runtime/vm/class_finalizer.h
@@ -54,11 +54,6 @@ class ClassFinalizer : public AllStatic {
 #if !defined(DART_PRECOMPILED_RUNTIME)
   // Mark [cls], its superclass and superinterfaces as can_be_future().
   static void MarkClassCanBeFuture(Zone* zone, const Class& cls);
-
-  // Mark [cls] and all its superclasses and superinterfaces as
-  // has_dynamically_extendable_subtypes().
-  static void MarkClassHasDynamicallyExtendableSubtypes(Zone* zone,
-                                                        const Class& cls);
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
   // Ensures members of the class are loaded, class layout is finalized and size

diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
@@ -1383,8 +1383,9 @@ void KernelLoader::LoadClass(const Library& library,
   if (HasPragma::decode(pragma_bits)) {
     out_class->set_has_pragma(true);
   }
-  if (DynModuleExtendablePragma::decode(pragma_bits)) {
-    out_class->set_is_dynamically_extendable(true);
+  if (DynModuleExtendablePragma::decode(pragma_bits) ||
+      DynModuleImplicitlyExtendablePragma::decode(pragma_bits)) {
+    out_class->set_has_dynamically_extendable_subtypes(true);
     IG->set_has_dynamically_extendable_classes(true);
   }
   class_helper.SetJustRead(ClassHelper::kAnnotations);
@@ -1622,8 +1623,6 @@ void KernelLoader::FinishClassLoading(const Class& klass,
     signature.set_result_type(T.ReceiverType(klass));
     function.set_has_pragma(HasPragma::decode(pragma_bits));
     function.set_is_visible(!InvisibleFunctionPragma::decode(pragma_bits));
-    function.SetIsDynamicallyOverridden(
-        DynModuleCanBeOverriddenPragma::decode(pragma_bits));
 
     FunctionNodeHelper function_node_helper(&helper_);
     function_node_helper.ReadUntilExcluding(
@@ -1806,11 +1805,21 @@ void KernelLoader::ReadVMAnnotations(intptr_t annotation_count,
                                              "dyn-module:extendable")) {
           *pragma_bits = DynModuleExtendablePragma::update(true, *pragma_bits);
         }
+        if (constant_reader.IsStringConstant(
+                name_index, "dyn-module:implicitly-extendable")) {
+          *pragma_bits =
+              DynModuleImplicitlyExtendablePragma::update(true, *pragma_bits);
+        }
         if (constant_reader.IsStringConstant(name_index,
                                              "dyn-module:can-be-overridden")) {
           *pragma_bits =
               DynModuleCanBeOverriddenPragma::update(true, *pragma_bits);
         }
+        if (constant_reader.IsStringConstant(
+                name_index, "dyn-module:can-be-overridden-implicitly")) {
+          *pragma_bits = DynModuleCanBeOverriddenImplicitlyPragma::update(
+              true, *pragma_bits);
+        }
       }
     } else {
       helper_.SkipExpression();
@@ -1877,7 +1886,8 @@ void KernelLoader::LoadProcedure(const Library& library,
                             is_synthetic);
   function.set_is_visible(!InvisibleFunctionPragma::decode(pragma_bits));
   function.SetIsDynamicallyOverridden(
-      DynModuleCanBeOverriddenPragma::decode(pragma_bits));
+      DynModuleCanBeOverriddenPragma::decode(pragma_bits) ||
+      DynModuleCanBeOverriddenImplicitlyPragma::decode(pragma_bits));
   if (register_function) {
     functions_.Add(&function);
   } else {

diff --git a/runtime/vm/kernel_loader.h b/runtime/vm/kernel_loader.h
@@ -238,8 +238,12 @@ class KernelLoader : public ValueObject {
   using SharedPragma = BitField<uint32_t, bool, FfiNativePragma::kNextBit>;
   using DynModuleExtendablePragma =
       BitField<uint32_t, bool, SharedPragma::kNextBit>;
-  using DynModuleCanBeOverriddenPragma =
+  using DynModuleImplicitlyExtendablePragma =
       BitField<uint32_t, bool, DynModuleExtendablePragma::kNextBit>;
+  using DynModuleCanBeOverriddenPragma =
+      BitField<uint32_t, bool, DynModuleImplicitlyExtendablePragma::kNextBit>;
+  using DynModuleCanBeOverriddenImplicitlyPragma =
+      BitField<uint32_t, bool, DynModuleCanBeOverriddenPragma::kNextBit>;
 
   void FinishTopLevelClassLoading(const Class& toplevel_class,
                                   const Library& library,

diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -3263,11 +3263,6 @@ void Class::set_can_be_future(bool value) const {
   set_state_bits(CanBeFutureBit::update(value, state_bits()));
 }
 
-void Class::set_is_dynamically_extendable(bool value) const {
-  ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter());
-  set_state_bits(IsDynamicallyExtendableBit::update(value, state_bits()));
-}
-
 void Class::set_has_dynamically_extendable_subtypes(bool value) const {
   ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter());
   set_state_bits(

diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -2116,13 +2116,9 @@ class Class : public Object {
   // It means that variable of static type based on this class may hold
   // a Future instance.
   using CanBeFutureBit = BitField<uint32_t, bool, IsFutureSubtypeBit::kNextBit>;
-  // This class can be extended, implemented or mixed-in by
-  // a dynamically loaded class.
-  using IsDynamicallyExtendableBit =
-      BitField<uint32_t, bool, CanBeFutureBit::kNextBit>;
   // This class has a dynamically extendable subtype.
   using HasDynamicallyExtendableSubtypesBit =
-      BitField<uint32_t, bool, IsDynamicallyExtendableBit::kNextBit>;
+      BitField<uint32_t, bool, CanBeFutureBit::kNextBit>;
   // This class was loaded from bytecode at runtime.
   using IsDeclaredInBytecodeBit =
       BitField<uint32_t, bool, HasDynamicallyExtendableSubtypesBit::kNextBit>;
@@ -2195,11 +2191,6 @@ class Class : public Object {
   void set_can_be_future(bool value) const;
   bool can_be_future() const { return CanBeFutureBit::decode(state_bits()); }
 
-  void set_is_dynamically_extendable(bool value) const;
-  bool is_dynamically_extendable() const {
-    return IsDynamicallyExtendableBit::decode(state_bits());
-  }
-
   void set_has_dynamically_extendable_subtypes(bool value) const;
   bool has_dynamically_extendable_subtypes() const {
     return HasDynamicallyExtendableSubtypesBit::decode(state_bits());