Skip to content

Commit 0f42d66

Browse files
alexmarkovCommit Queue
authored andcommitted
[dyn_modules] Fix overriding of a method declared in a mixin
Mixin transformation copies members of a mixin into a mixin application class. Copied members should still behave similarly to original members in terms of overriding. When validating a dynamic module, overriding a copy of mixin member should be allowed if original member can be overridden. When annotating AST with dynamic interface pragmas, can-be-overridden pragmas should apply to all copies of mixin members. TEST=pkg/dynamic_modules/test/data/override_mixin_method Fixes b/418681054 Change-Id: I038ff2133ef6f0c288a47cec7300c6ac4b23cbab Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429423 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 9189341 commit 0f42d66

File tree

7 files changed

+146
-1
lines changed

7 files changed

+146
-1
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
# for details. All rights reserved. Use of this source code is governed by a
3+
# BSD-style license that can be found in the LICENSE file.
4+
extendable:
5+
- library: 'shared/shared.dart'
6+
7+
callable:
8+
- library: 'shared/shared.dart'
9+
10+
can-be-overridden:
11+
- library: 'shared/shared.dart'
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import '../../common/testing.dart' as helper;
6+
import 'package:expect/expect.dart';
7+
8+
import 'shared/shared.dart';
9+
10+
/// A dynamic module can override a method declared in a mixin.
11+
void main() async {
12+
final objs = (await helper.load('entry1.dart')) as List<A>;
13+
final o1 = objs[0];
14+
Expect.equals('DynA1.foo, super: M2.foo', o1.foo());
15+
Expect.equals('M2.bar', o1.bar());
16+
final o2 = objs[1];
17+
Expect.equals('M2.foo', o2.foo());
18+
Expect.equals('M3.bar', o2.bar());
19+
helper.done();
20+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import '../shared/shared.dart';
6+
7+
class DynA1 extends A {
8+
@override
9+
String foo() => 'DynA1.foo, super: ${super.foo()}';
10+
}
11+
12+
class DynA2 extends A with M3 {}
13+
14+
@pragma('dyn-module:entry-point')
15+
List<A> dynamicModuleEntrypoint() {
16+
return <A>[DynA1(), DynA2()];
17+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
class A extends B with M1, M2 {}
6+
7+
class B {
8+
String bar() => 'B.bar';
9+
}
10+
11+
mixin M1 on B {}
12+
13+
mixin M2 on B, M1 {
14+
String foo() => 'M2.foo';
15+
16+
@override
17+
String bar() => 'M2.bar';
18+
}
19+
20+
mixin M3 on B {
21+
@override
22+
String bar() => 'M3.bar';
23+
}

pkg/front_end/lib/src/kernel/dynamic_module_validator.dart

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,9 @@ class _DynamicModuleValidator extends RecursiveVisitor {
616616
if (target is Procedure) {
617617
target = _unwrapMixinStubs(target);
618618
}
619+
if (target is Member) {
620+
target = _unwrapMixinCopy(target);
621+
}
619622
if (!_isFromDynamicModule(target) &&
620623
!_isSpecified(target, spec.callable) &&
621624
!languageImplPragmas.isCallable(target)) {
@@ -692,6 +695,22 @@ class _DynamicModuleValidator extends RecursiveVisitor {
692695
return member;
693696
}
694697

698+
// Unwrap copied method from an eliminated mixin to get actual
699+
// interface member.
700+
Member _unwrapMixinCopy(Member member) {
701+
if (!member.isInstanceMember) {
702+
return member;
703+
}
704+
Class enclosingClass = member.enclosingClass!;
705+
if (!enclosingClass.isEliminatedMixin) {
706+
return member;
707+
}
708+
// Coverage-ignore-block(suite): Not run.
709+
Class origin = enclosingClass.implementedTypes.last.classNode;
710+
return hierarchy.getInterfaceMember(origin, member.name,
711+
setter: member is Procedure && member.isSetter)!;
712+
}
713+
695714
void _verifyOverrides(
696715
List<Member> implementationMembers, List<Member> interfaceMembers) {
697716
int i = 0, j = 0;
@@ -716,6 +735,7 @@ class _DynamicModuleValidator extends RecursiveVisitor {
716735
}
717736

718737
void _verifyOverride(Member ownMember, Member superMember) {
738+
superMember = _unwrapMixinCopy(superMember);
719739
if (!_isFromDynamicModule(superMember) &&
720740
!_isSpecified(superMember, spec.canBeOverridden) &&
721741
!languageImplPragmas.canBeOverridden(superMember)) {

pkg/front_end/test/coverage_suite_expected.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
730730
),
731731
// 100.0%.
732732
"package:front_end/src/kernel/dynamic_module_validator.dart": (
733-
hitCount: 404,
733+
hitCount: 411,
734734
missCount: 0,
735735
),
736736
// 100.0%.

pkg/vm/lib/transformations/dynamic_interface_annotator.dart

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ void annotateComponent(
6666
);
6767

6868
final hierarchy = ClassHierarchy(component, coreTypes);
69+
70+
copyCanBeOverriddenToMixinApplications(
71+
canBeOverriddenAnnotator.annotatedMembers,
72+
canBeOverriddenAnnotator.pragma,
73+
component,
74+
hierarchy,
75+
);
76+
6977
_ImplicitOverridesAnnotator(
7078
pragmaConstant(coreTypes, kDynModuleCanBeOverriddenImplicitlyPragmaName),
7179
hierarchy,
@@ -630,3 +638,49 @@ class _DiscoverLanguageImplPragmasVisitor extends RecursiveVisitor {
630638
}
631639
}
632640
}
641+
642+
// Mixin transformation copies all members of mixins into mixin applications.
643+
// So we need to copy can-be-overridden pragmas from members of mixins to
644+
// their copies in the transformed mixin applications.
645+
void copyCanBeOverriddenToMixinApplications(
646+
Set<Member> overriddenMembers,
647+
Constant pragma,
648+
Component component,
649+
ClassHierarchy hierarchy,
650+
) {
651+
void processMember(Member original, Class mixinApplication) {
652+
if (!original.isInstanceMember) {
653+
return;
654+
}
655+
if (!overriddenMembers.contains(original)) {
656+
return;
657+
}
658+
final member = ClassHierarchy.findMemberByName(
659+
hierarchy.getDeclaredMembers(
660+
mixinApplication,
661+
setters: original is Procedure && original.isSetter,
662+
),
663+
original.name,
664+
);
665+
if (member == null) {
666+
return;
667+
}
668+
if (overriddenMembers.add(member)) {
669+
member.addAnnotation(ConstantExpression(pragma));
670+
}
671+
}
672+
673+
for (final library in component.libraries) {
674+
for (final cls in library.classes) {
675+
if (cls.isEliminatedMixin) {
676+
final origin = cls.implementedTypes.last.classNode;
677+
for (final proc in origin.procedures) {
678+
processMember(proc, cls);
679+
}
680+
for (final field in origin.fields) {
681+
processMember(field, cls);
682+
}
683+
}
684+
}
685+
}
686+
}

0 commit comments

Comments
 (0)