Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix
  • Loading branch information
christopherfujino committed Dec 8, 2022
commit f013067f4b0c4af30adefd60bc00c5948b8b6dba
29 changes: 28 additions & 1 deletion tools/const_finder/lib/const_finder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,31 @@ class _ConstVisitor extends RecursiveVisitor<void> {

bool inIgnoredClass = false;

/// Whether or not we are currently within the declaration of the target class.
///
/// We use this to determine when to skip tracking non-constant
/// [ConstructorInvocation]s. This is because, in web builds, a static
/// method is always created called _#new#tearOff() which returns the result
/// of a non-constant invocation of the unnamed constructor.
///
/// For the following Dart class "FooBar":
///
/// class FooBar {
/// const FooBar();
/// }
///
/// The following kernel structure is generated:
///
/// class FooBar extends core::Object /*hasConstConstructor*/ {
/// const constructor •() → min::FooBar
/// : super core::Object::•()
/// ;
/// static method _#new#tearOff() → min::FooBar
/// return new min::FooBar::•(); /* this is a non-const constructor invocation */
/// method noOp() → void {}
/// }
bool inTargetClass = false;

/// The name of the name of the class of the annotation marking classes
/// whose constant references should be ignored.
final String? annotationClassName;
Expand Down Expand Up @@ -73,7 +98,7 @@ class _ConstVisitor extends RecursiveVisitor<void> {
@override
void visitConstructorInvocation(ConstructorInvocation node) {
final Class parentClass = node.target.parent! as Class;
if (_matches(parentClass)) {
if (_matches(parentClass) && !inTargetClass) {
final Location location = node.location!;
nonConstantLocations.add(<String, dynamic>{
'file': location.file.toString(),
Expand All @@ -86,9 +111,11 @@ class _ConstVisitor extends RecursiveVisitor<void> {

@override
void visitClass(Class node) {
inTargetClass = _matches(node);
// check if this is a class that we should ignore
inIgnoredClass = _classShouldBeIgnored(node);
super.visitClass(node);
inTargetClass = false;
inIgnoredClass = false;
}

Expand Down
61 changes: 52 additions & 9 deletions tools/const_finder/test/const_finder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ void expectInstances(dynamic value, dynamic expected, Compiler compiler) {

final Equality<Object?> equality;
if (compiler == Compiler.dart2js) {
// Ignore comparing nonConstantLocations in web dills because it will have
// extra fields.
(value as Map<String, Object?>).remove('nonConstantLocations');
(expected as Map<String, Object?>).remove('nonConstantLocations');
equality = const Dart2JSDeepCollectionEquality();
} else {
equality = const DeepCollectionEquality();
Expand Down Expand Up @@ -69,9 +65,7 @@ void _checkConsts(String dillPath, Compiler compiler) {
classLibraryUri: 'package:const_finder_fixtures/target.dart',
className: 'Target',
);
expectInstances(
finder.findInstances(),
<String, dynamic>{
final Map<String, Object?> expectation = <String, dynamic>{
'constantInstances': <Map<String, dynamic>>[
<String, dynamic>{'stringValue': '100', 'intValue': 100, 'targetValue': null},
<String, dynamic>{'stringValue': '102', 'intValue': 102, 'targetValue': null},
Expand All @@ -95,7 +89,27 @@ void _checkConsts(String dillPath, Compiler compiler) {
<String, dynamic>{'stringValue': 'package', 'intValue':-1, 'targetValue': null},
],
'nonConstantLocations': <dynamic>[],
},
};
if (compiler == Compiler.aot) {
expectation['nonConstantLocations'] = <Object?>[];
} else {
// Without true tree-shaking, there is a non-const reference in a
// never-invoked function that will be present in the dill.
final String fixturesUrl = Platform.isWindows
? '/$fixtures'.replaceAll(Platform.pathSeparator, '/')
: fixtures;

expectation['nonConstantLocations'] = <Object?>[
<String, dynamic>{
'file': 'file://$fixturesUrl/pkg/package.dart',
'line': 14,
'column': 25,
},
];
}
expectInstances(
finder.findInstances(),
expectation,
compiler,
);

Expand Down Expand Up @@ -212,6 +226,9 @@ void _checkNonConstsWeb(String dillPath, Compiler compiler) {
className: 'Target',
);

final String fixturesUrl = Platform.isWindows
? '/$fixtures'.replaceAll(Platform.pathSeparator, '/')
: fixtures;
expectInstances(
finder.findInstances(),
<String, dynamic>{
Expand All @@ -225,7 +242,33 @@ void _checkNonConstsWeb(String dillPath, Compiler compiler) {
<String, dynamic>{'stringValue': '7', 'intValue': 7, 'targetValue': null},
<String, dynamic>{'stringValue': 'package', 'intValue': -1, 'targetValue': null},
],
'nonConstantLocations': <dynamic>[]
'nonConstantLocations': <dynamic>[
<String, dynamic>{
'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
'line': 14,
'column': 26,
},
<String, dynamic>{
'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
'line': 16,
'column': 26,
},
<String, dynamic>{
'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
'line': 16,
'column': 41,
},
<String, dynamic>{
'file': 'file://$fixturesUrl/lib/consts_and_non.dart',
'line': 17,
'column': 26,
},
<String, dynamic>{
'file': 'file://$fixturesUrl/pkg/package.dart',
'line': 14,
'column': 25,
}
],
},
compiler,
);
Expand Down