Skip to content

Commit 0064249

Browse files
sigmundchCommit Queue
authored andcommitted
[trim] relax language version requirement.
Only enforce that a library must be 3.0 or higher if it contains extendable elements. Otherwise, we can ignore legacy mixin classes and trim the entire library. TESTED=apply_mixin case in dynamic_modules tests. Change-Id: I1378c6d4678305ae634b4259e7eb28a48327d645 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425187 Reviewed-by: Alexander Markov <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Sigmund Cherem <[email protected]>
1 parent ff87171 commit 0064249

File tree

5 files changed

+63
-30
lines changed

5 files changed

+63
-30
lines changed

pkg/dynamic_modules/test/data/apply_mixin/dynamic_interface.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ extendable:
1010
callable:
1111
- library: 'shared/shared.dart'
1212
class: 'M'
13+
- library: 'shared/shared_old.dart'
14+
class: 'M3'
1315

1416
can-be-overridden:
1517
- library: 'shared/shared.dart'

pkg/dynamic_modules/test/data/apply_mixin/modules/entry1.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,7 @@ class Child extends RealA with M {
1515
}
1616

1717
@pragma('dyn-module:entry-point')
18-
Object? dynamicModuleEntrypoint() => Child();
18+
Object? dynamicModuleEntrypoint() {
19+
M3().method3();
20+
return Child();
21+
}

pkg/dynamic_modules/test/data/apply_mixin/shared/shared.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
export 'shared_old.dart';
6+
57
abstract interface class A {
68
int method1();
79
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
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+
// @dart = 2.19
6+
7+
// This library is purposely labeled to use an OLD sdk version prior to 3.0.
8+
// The trim script based on dynamic interfaces will reject uses of `M3` in the
9+
// extendable section, but allows it on the callable section.
10+
class M3 {
11+
String method3() => '3';
12+
}

pkg/front_end/lib/src/util/trim.dart

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,16 @@ import 'package:front_end/src/kernel/dynamic_module_validator.dart';
3636
Future<void> createTrimmedCopy(TrimOptions options) async {
3737
Component component = loadComponentFromBinary(options.inputPlatformPath);
3838
loadComponentFromBinary(options.inputAppPath, component);
39-
Set<Library> included = {};
40-
41-
// Validate version and clear method bodies.
42-
component.accept(new Trimmer(options.librariesToClear));
43-
39+
bool Function(Library) isExtendable;
40+
bool Function(Library) isRoot;
4441
if (options.dynamicInterfaceContents == null) {
45-
// Transitively include all libraries from a set of user libraries.
46-
addReachable(
47-
component.libraries,
48-
(Library lib) =>
49-
_isRootFromPatterns(lib, options.requiredUserLibraries),
50-
included);
42+
// Include all libraries from a set of user libraries.
43+
isExtendable = (lib) => true;
44+
isRoot = (Library lib) =>
45+
_isRootFromPatterns(lib, options.requiredUserLibraries);
5146
} else {
52-
// Transitively include all libraries declared as accessible from the
53-
// dynamic_interface specification.
47+
// Include all libraries declared as accessible from the dynamic_interface
48+
// specification.
5449
DynamicInterfaceSpecification spec = new DynamicInterfaceSpecification(
5550
options.dynamicInterfaceContents!,
5651
options.dynamicInterfaceUri!,
@@ -61,15 +56,24 @@ Future<void> createTrimmedCopy(TrimOptions options) async {
6156
Library() => node,
6257
_ => throw 'Unexpected node ${node.runtimeType} $node'
6358
};
59+
Set<Library> extendableLibraries =
60+
spec.extendable.map(enclosingLibrary).toSet();
6461
Set<Library> roots = {
6562
...spec.callable.map(enclosingLibrary),
66-
...spec.extendable.map(enclosingLibrary),
63+
...extendableLibraries,
6764
...spec.canBeOverridden.map(enclosingLibrary),
6865
};
69-
addReachable(component.libraries, roots.contains, included);
66+
isExtendable = extendableLibraries.contains;
67+
isRoot = roots.contains;
7068
}
69+
Set<Library> included = {};
70+
// Validate version and clear method bodies.
71+
component.accept(new Trimmer(options.librariesToClear, isExtendable));
72+
73+
// Include all root libraries according to flags or dynamic interface.
74+
addReachable(component.libraries, isRoot, included);
7175

72-
// Transitively include libraries needed by the required platform libraries.
76+
// Also include libraries needed by the required platform libraries.
7377
addReachable(
7478
component.libraries,
7579
(Library lib) => _isRootFromPatterns(lib, options.requiredDartLibraries),
@@ -115,8 +119,9 @@ bool _isRootFromPatterns(Library lib, Set<String> patterns) {
115119
return false;
116120
}
117121

118-
/// Validates that all libraries are 3.0 or higher, then trims contents
119-
/// as much as possible, while enabling modular compilation later on.
122+
/// Validates that all libraries with extendable classes are 3.0 or higher, then
123+
/// trims contents as much as possible, while enabling modular compilation later
124+
/// on.
120125
///
121126
/// Currently we:
122127
/// * deletes bodies of constructors and procedures, except when deemed
@@ -134,19 +139,27 @@ class Trimmer extends RecursiveVisitor {
134139
/// the library node itself.
135140
final Set<String> librariesToClear;
136141

137-
/// Whether we are within a mixin declaration, and hence method bodies need to
138-
/// be preserved.
139-
bool withinMixin = false;
142+
/// Subset of libraries that may contain extendable classes according to the
143+
/// dynamic interface (defaults to all libraries if the dynamic interface is
144+
/// not provided).
145+
///
146+
/// Used by the trimmer to determine whether legacy mixins may be at play in
147+
/// the dynamic interface.
148+
final bool Function(Library) isExtendable;
149+
150+
/// Whether we are within a mixin declaration in an extendable library, and
151+
/// hence method bodies need to be preserved.
152+
bool preserveMethodBodies = false;
140153

141-
Trimmer(this.librariesToClear);
154+
Trimmer(this.librariesToClear, this.isExtendable);
142155

143156
@override
144157
void visitLibrary(Library node) {
145158
Uri uri = node.importUri;
146-
if (node.languageVersion.major < 3) {
147-
print(
148-
'Error: Library "$uri" has version ${node.languageVersion.toText()}, '
149-
'which is older than 3.0');
159+
if (isExtendable(node) && node.languageVersion.major < 3) {
160+
print('Error: Only libraries 3.0 or newer may be used for extendable '
161+
'classes. The library `"$uri" includes extendable classes, but has '
162+
'version ${node.languageVersion.toText()}, which is older than 3.0.');
150163
exit(1);
151164
}
152165

@@ -168,9 +181,10 @@ class Trimmer extends RecursiveVisitor {
168181

169182
@override
170183
void visitClass(Class node) {
171-
withinMixin = node.isMixinClass || node.isMixinDeclaration;
184+
preserveMethodBodies = isExtendable(node.enclosingLibrary) &&
185+
(node.isMixinClass || node.isMixinDeclaration);
172186
super.visitClass(node);
173-
withinMixin = false;
187+
preserveMethodBodies = false;
174188
}
175189

176190
@override
@@ -190,7 +204,7 @@ class Trimmer extends RecursiveVisitor {
190204
void visitProcedure(Procedure node) {
191205
// Preserve method bodies of mixin declarations, these are copied when
192206
// mixins are applied in subtypes.
193-
if (!withinMixin) {
207+
if (!preserveMethodBodies) {
194208
node.function.body = null;
195209
}
196210
}

0 commit comments

Comments
 (0)