[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 https://github.com/dart-lang/sdk/issues/59880
Change-Id: Id4570cc86303f8e45a061d696e9bca0d0b2b4b81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403951
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
diff --git a/pkg/vm/lib/transformations/dynamic_interface_annotator.dart b/pkg/vm/lib/transformations/dynamic_interface_annotator.dart
index 3da52ca..66734dc 100644
--- 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 @@
kDynModuleCallablePragmaName,
kDynModuleExtendablePragmaName,
kDynModuleImplicitlyCallablePragmaName,
- kDynModuleImplicitlyExtendablePragmaName;
+ kDynModuleImplicitlyExtendablePragmaName,
+ kDynModuleCanBeOverriddenImplicitlyPragmaName;
const bool _debug = false;
@@ -33,13 +35,28 @@
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,20 +188,77 @@
}
}
-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);
+ }
+ }
+}
+
class _ImplicitUsesAnnotator extends RecursiveVisitor {
final Constant pragma;
diff --git a/pkg/vm/lib/transformations/pragma.dart b/pkg/vm/lib/transformations/pragma.dart
index 152b8ad..f98aaf2 100644
--- a/pkg/vm/lib/transformations/pragma.dart
+++ b/pkg/vm/lib/transformations/pragma.dart
@@ -26,6 +26,8 @@
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
index c4d2f14..fb9265b 100644
--- 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 @@
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
index c23fe99..9d0891f 100644
--- 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 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
index 7d07468..c76db5a 100644
--- 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 @@
}
@#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 @@
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 @@
#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
index 25190b6..76ad80c 100644
--- 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 @@
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
index 5a13e50..b63685c 100644
--- a/runtime/vm/class_finalizer.cc
+++ b/runtime/vm/class_finalizer.cc
@@ -589,10 +589,6 @@
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::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
index c575c9e..7253df7 100644
--- a/runtime/vm/class_finalizer.h
+++ b/runtime/vm/class_finalizer.h
@@ -54,11 +54,6 @@
#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
index 34b16ac..1c0c7b7 100644
--- a/runtime/vm/kernel_loader.cc
+++ b/runtime/vm/kernel_loader.cc
@@ -1383,8 +1383,9 @@
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 @@
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 @@
"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 @@
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
index 537a3f4..08baaec 100644
--- a/runtime/vm/kernel_loader.h
+++ b/runtime/vm/kernel_loader.h
@@ -238,8 +238,12 @@
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
index fd57986..9d47c10 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -3263,11 +3263,6 @@
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
index fa2bf63..e0c8387 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -2116,13 +2116,9 @@
// 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 @@
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());
