From 09240e4302da4ef8a064c7ce0a166c03b4ec74ef Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 21 Jan 2020 18:53:15 -0800 Subject: [PATCH 01/10] Fix issues in const_finder --- tools/const_finder/bin/main.dart | 5 --- tools/const_finder/lib/const_finder.dart | 40 ++----------------- .../const_finder/test/const_finder_test.dart | 2 - 3 files changed, 4 insertions(+), 43 deletions(-) diff --git a/tools/const_finder/bin/main.dart b/tools/const_finder/bin/main.dart index d29a957118983..3d0cdf4d89121 100644 --- a/tools/const_finder/bin/main.dart +++ b/tools/const_finder/bin/main.dart @@ -43,10 +43,6 @@ void main(List args) { valueHelp: 'path/to/main.dill', help: 'The path to a kernel file to parse, which was created from the ' 'main-package-uri library.') - ..addOption('main-library-uri', - help: 'The package: URI to treat as the main entrypoint library ' - '(the same package used to create the kernel-file).', - valueHelp: 'package:hello_world/main.dart') ..addOption('class-library-uri', help: 'The package: URI of the class to find.', valueHelp: 'package:flutter/src/widgets/icon_data.dart') @@ -73,7 +69,6 @@ void main(List args) { final ConstFinder finder = ConstFinder( kernelFilePath: getArg('kernel-file'), - targetLibraryUri: getArg('main-library-uri'), classLibraryUri: getArg('class-library-uri'), className: getArg('class-name'), ); diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart index b40ac249d9901..13f58dde948e2 100644 --- a/tools/const_finder/lib/const_finder.dart +++ b/tools/const_finder/lib/const_finder.dart @@ -8,11 +8,9 @@ import 'package:meta/meta.dart'; class _ConstVisitor extends RecursiveVisitor { _ConstVisitor( this.kernelFilePath, - this.targetLibraryUri, this.classLibraryUri, this.className, ) : assert(kernelFilePath != null), - assert(targetLibraryUri != null), assert(classLibraryUri != null), assert(className != null), constantInstances = >[], @@ -21,9 +19,6 @@ class _ConstVisitor extends RecursiveVisitor { /// The path to the file to open. final String kernelFilePath; - /// The library URI for the main entrypoint of the target library. - final String targetLibraryUri; - /// The library URI for the class to find. final String classLibraryUri; @@ -42,7 +37,7 @@ class _ConstVisitor extends RecursiveVisitor { void visitConstructorInvocation(ConstructorInvocation node) { final Class parentClass = node.target.parent as Class; if (!_matches(parentClass)) { - super.visitConstructorInvocation(node); + return super.visitConstructorInvocation(node); } nonConstantLocations.add({ 'file': node.location.file.toString(), @@ -71,53 +66,26 @@ class ConstFinder { /// be null. /// /// The `kernelFilePath` is the path to a dill (kernel) file to process. - /// - /// The `targetLibraryUri` is the `package:` URI of the main entrypoint to - /// search from. - /// - /// - /// ConstFinder({ @required String kernelFilePath, - @required String targetLibraryUri, @required String classLibraryUri, @required String className, }) : _visitor = _ConstVisitor( kernelFilePath, - targetLibraryUri, classLibraryUri, className, ); final _ConstVisitor _visitor; - Library _getRoot() { - final Component binary = loadComponentFromBinary(_visitor.kernelFilePath); - return binary.libraries.firstWhere( - (Library library) => library.canonicalName.name == _visitor.targetLibraryUri, - orElse: () => throw LibraryNotFoundException._(_visitor.targetLibraryUri), - ); - } - /// Finds all instances Map findInstances() { - final Library root = _getRoot(); - root.visitChildren(_visitor); + for (Library library in loadComponentFromBinary(_visitor.kernelFilePath).libraries) { + library.visitChildren(_visitor); + } return { 'constantInstances': _visitor.constantInstances, 'nonConstantLocations': _visitor.nonConstantLocations, }; } } - -/// Exception thrown by [ConstFinder.findInstances] when the target library -/// is not found. -class LibraryNotFoundException implements Exception { - const LibraryNotFoundException._(this.targetLibraryUri); - - /// The library target URI that could not be found. - final String targetLibraryUri; - - @override - String toString() => 'Could not find target library for "$targetLibraryUri".'; -} diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index cc9a2761755f8..478c366070d6f 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -34,7 +34,6 @@ void _checkConsts() { print('Checking for expected constants.'); final ConstFinder finder = ConstFinder( kernelFilePath: constsDill, - targetLibraryUri: 'package:const_finder_fixtures/consts.dart', classLibraryUri: 'package:const_finder_fixtures/target.dart', className: 'Target', ); @@ -55,7 +54,6 @@ void _checkNonConsts() { print('Checking for non-constant instances.'); final ConstFinder finder = ConstFinder( kernelFilePath: constsAndNonDill, - targetLibraryUri: 'package:const_finder_fixtures/consts_and_non.dart', classLibraryUri: 'package:const_finder_fixtures/target.dart', className: 'Target', ); From 67643232aa131132c36efc62aed4022d880e8861 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 21 Jan 2020 18:57:36 -0800 Subject: [PATCH 02/10] test --- tools/const_finder/test/fixtures/lib/consts.dart | 6 ++++++ tools/const_finder/test/fixtures/lib/consts_and_non.dart | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart index ef1969c491deb..39670d989b06f 100644 --- a/tools/const_finder/test/fixtures/lib/consts.dart +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -13,5 +13,11 @@ void main() { const Target target3 = Target('3', 3); // should be tree shaken out. target1.hit(); target2.hit(); + + const IgnoreMe ignoreMe = IgnoreMe(); // Should be ignored. + print(ignoreMe); } +class IgnoreMe { + const IgnoreMe(); +} diff --git a/tools/const_finder/test/fixtures/lib/consts_and_non.dart b/tools/const_finder/test/fixtures/lib/consts_and_non.dart index d717faaeae3ae..727b22992db45 100644 --- a/tools/const_finder/test/fixtures/lib/consts_and_non.dart +++ b/tools/const_finder/test/fixtures/lib/consts_and_non.dart @@ -14,4 +14,11 @@ void main() { final Target target3 = Target('3', 3); // should be tree shaken out. target1.hit(); target2.hit(); + + const IgnoreMe ignoreMe = IgnoreMe(); // Should be ignored. + print(ignoreMe); +} + +class IgnoreMe { + const IgnoreMe(); } From 2f7c065bda1023601adedbff365c9787082e462d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 21 Jan 2020 18:59:08 -0800 Subject: [PATCH 03/10] void return --- tools/const_finder/lib/const_finder.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart index 13f58dde948e2..f739c7867fda8 100644 --- a/tools/const_finder/lib/const_finder.dart +++ b/tools/const_finder/lib/const_finder.dart @@ -37,7 +37,8 @@ class _ConstVisitor extends RecursiveVisitor { void visitConstructorInvocation(ConstructorInvocation node) { final Class parentClass = node.target.parent as Class; if (!_matches(parentClass)) { - return super.visitConstructorInvocation(node); + super.visitConstructorInvocation(node); + return; } nonConstantLocations.add({ 'file': node.location.file.toString(), From b3a6d4a335df180304b286ee7a441bba5c54d1a3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jan 2020 00:32:43 -0800 Subject: [PATCH 04/10] more --- tools/const_finder/lib/const_finder.dart | 12 ++++++++++- .../const_finder/test/const_finder_test.dart | 18 ++++++++++++---- .../test/fixtures/lib/consts.dart | 18 +++++++++++----- .../test/fixtures/lib/consts_and_non.dart | 21 ++++++++++++++----- .../test/fixtures/lib/target.dart | 3 ++- 5 files changed, 56 insertions(+), 16 deletions(-) diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart index f739c7867fda8..1eb8776a91f40 100644 --- a/tools/const_finder/lib/const_finder.dart +++ b/tools/const_finder/lib/const_finder.dart @@ -13,6 +13,7 @@ class _ConstVisitor extends RecursiveVisitor { ) : assert(kernelFilePath != null), assert(classLibraryUri != null), assert(className != null), + _visitedInstnaces = {}, constantInstances = >[], nonConstantLocations = >[]; @@ -25,6 +26,7 @@ class _ConstVisitor extends RecursiveVisitor { /// The name of the class to find. final String className; + final Set _visitedInstnaces; final List> constantInstances; final List> nonConstantLocations; @@ -49,15 +51,22 @@ class _ConstVisitor extends RecursiveVisitor { @override void visitInstanceConstantReference(InstanceConstant node) { + node.fieldValues.values.whereType().forEach(visitInstanceConstantReference); if (!_matches(node.classNode)) { + super.visitInstanceConstantReference(node); return; } final Map instance = {}; for (MapEntry kvp in node.fieldValues.entries) { + if (kvp.value is! PrimitiveConstant) { + continue; + } final PrimitiveConstant value = kvp.value as PrimitiveConstant; instance[kvp.key.canonicalName.name] = value.value; } - constantInstances.add(instance); + if (_visitedInstnaces.add(instance.toString())) { + constantInstances.add(instance); + } } } @@ -81,6 +90,7 @@ class ConstFinder { /// Finds all instances Map findInstances() { + _visitor._visitedInstnaces.clear(); for (Library library in loadComponentFromBinary(_visitor.kernelFilePath).libraries) { library.visitChildren(_visitor); } diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index 478c366070d6f..ba115ed1194e4 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -42,8 +42,11 @@ void _checkConsts() { jsonEncode(finder.findInstances()), jsonEncode({ 'constantInstances': >[ - {'stringValue': '1', 'intValue': 1}, - {'stringValue': '2', 'intValue': 2} + {'stringValue': '1', 'intValue': 1, 'targetValue': null}, + {'stringValue': '4', 'intValue': 4, 'targetValue': null}, + {'stringValue': '2', 'intValue': 2}, + {'stringValue': '6', 'intValue': 6, 'targetValue': null}, + {'stringValue': '7', 'intValue': 7, 'targetValue': null}, ], 'nonConstantLocations': [], }), @@ -62,7 +65,9 @@ void _checkNonConsts() { jsonEncode(finder.findInstances()), jsonEncode({ 'constantInstances': [ - {'stringValue': '1', 'intValue': 1} + {'stringValue': '1', 'intValue': 1, 'targetValue': null}, + {'stringValue': '6', 'intValue': 6, 'targetValue': null}, + {'stringValue': '7', 'intValue': 7, 'targetValue': null}, ], 'nonConstantLocations': [ { @@ -72,7 +77,12 @@ void _checkNonConsts() { }, { 'file': 'file://$fixtures/lib/consts_and_non.dart', - 'line': 14, + 'line': 15, + 'column': 26 + }, + { + 'file': 'file://$fixtures/lib/consts_and_non.dart', + 'line': 17, 'column': 26 }, ] diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart index 39670d989b06f..b59e4a19ef3fc 100644 --- a/tools/const_finder/test/fixtures/lib/consts.dart +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -7,17 +7,25 @@ import 'dart:core'; import 'target.dart'; void main() { - const Target target1 = Target('1', 1); - const Target target2 = Target('2', 2); + const Target target1 = Target('1', 1, null); + const Target target2 = Target('2', 2, Target('4', 4, null)); // ignore: unused_local_variable - const Target target3 = Target('3', 3); // should be tree shaken out. + const Target target3 = Target('3', 3, Target('5', 5, null)); // should be tree shaken out. target1.hit(); target2.hit(); - const IgnoreMe ignoreMe = IgnoreMe(); // Should be ignored. + blah(const Target('6', 6, null)); + + const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not. print(ignoreMe); } class IgnoreMe { - const IgnoreMe(); + const IgnoreMe([this.target]); + + final Target target; +} + +void blah(Target target) { + print(target); } diff --git a/tools/const_finder/test/fixtures/lib/consts_and_non.dart b/tools/const_finder/test/fixtures/lib/consts_and_non.dart index 727b22992db45..2d8a0bbe8913f 100644 --- a/tools/const_finder/test/fixtures/lib/consts_and_non.dart +++ b/tools/const_finder/test/fixtures/lib/consts_and_non.dart @@ -8,17 +8,28 @@ import 'dart:core'; import 'target.dart'; void main() { - const Target target1 = Target('1', 1); - final Target target2 = Target('2', 2); + const Target target1 = Target('1', 1, null); + final Target target2 = Target('2', 2, const Target('4', 4, null)); + + // ignore: unused_local_variable + final Target target3 = Target('3', 3, Target('5', 5, null)); // should be tree shaken out. // ignore: unused_local_variable - final Target target3 = Target('3', 3); // should be tree shaken out. + final Target target6 = Target('6', 6, null); // should be tree shaken out. target1.hit(); target2.hit(); - const IgnoreMe ignoreMe = IgnoreMe(); // Should be ignored. + blah(const Target('6', 6, null)); + + const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not. print(ignoreMe); } class IgnoreMe { - const IgnoreMe(); + const IgnoreMe([this.target]); + + final Target target; +} + +void blah(Target target) { + print(target); } diff --git a/tools/const_finder/test/fixtures/lib/target.dart b/tools/const_finder/test/fixtures/lib/target.dart index 779fa8691c334..4046216970ed9 100644 --- a/tools/const_finder/test/fixtures/lib/target.dart +++ b/tools/const_finder/test/fixtures/lib/target.dart @@ -3,10 +3,11 @@ // found in the LICENSE file. class Target { - const Target(this.stringValue, this.intValue); + const Target(this.stringValue, this.intValue, this.targetValue); final String stringValue; final int intValue; + final Target targetValue; void hit() { print('$stringValue $intValue'); From 3f869e23a9dfbbb755a18e69501695a5e524ed37 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jan 2020 07:39:40 -0800 Subject: [PATCH 05/10] test more cases --- tools/const_finder/test/const_finder_test.dart | 6 ++++++ tools/const_finder/test/fixtures/lib/consts.dart | 6 ++++++ tools/const_finder/test/fixtures/lib/consts_and_non.dart | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index ba115ed1194e4..9ac9e7c376f12 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -46,6 +46,9 @@ void _checkConsts() { {'stringValue': '4', 'intValue': 4, 'targetValue': null}, {'stringValue': '2', 'intValue': 2}, {'stringValue': '6', 'intValue': 6, 'targetValue': null}, + {'stringValue': '8', 'intValue': 8, 'targetValue': null}, + {'stringValue': '10', 'intValue': 10, 'targetValue': null}, + {'stringValue': '9', 'intValue': 9}, {'stringValue': '7', 'intValue': 7, 'targetValue': null}, ], 'nonConstantLocations': [], @@ -67,6 +70,9 @@ void _checkNonConsts() { 'constantInstances': [ {'stringValue': '1', 'intValue': 1, 'targetValue': null}, {'stringValue': '6', 'intValue': 6, 'targetValue': null}, + {'stringValue': '8', 'intValue': 8, 'targetValue': null}, + {'stringValue': '10', 'intValue': 10, 'targetValue': null}, + {'stringValue': '9', 'intValue': 9}, {'stringValue': '7', 'intValue': 7, 'targetValue': null}, ], 'nonConstantLocations': [ diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart index b59e4a19ef3fc..63176e8dfe289 100644 --- a/tools/const_finder/test/fixtures/lib/consts.dart +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -17,7 +17,13 @@ void main() { blah(const Target('6', 6, null)); const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not. + // ignore: prefer_const_constructors + final IgnoreMe ignoreMe2 = IgnoreMe(const Target('8', 8, null)); + // ignore: prefer_const_constructors + final IgnoreMe ignoreMe3 = IgnoreMe(const Target('9', 9, Target('10', 10, null))); print(ignoreMe); + print(ignoreMe2); + print(ignoreMe3); } class IgnoreMe { diff --git a/tools/const_finder/test/fixtures/lib/consts_and_non.dart b/tools/const_finder/test/fixtures/lib/consts_and_non.dart index 2d8a0bbe8913f..9c832fa263170 100644 --- a/tools/const_finder/test/fixtures/lib/consts_and_non.dart +++ b/tools/const_finder/test/fixtures/lib/consts_and_non.dart @@ -21,7 +21,13 @@ void main() { blah(const Target('6', 6, null)); const IgnoreMe ignoreMe = IgnoreMe(Target('7', 7, null)); // IgnoreMe is ignored but 7 is not. + // ignore: prefer_const_constructors + final IgnoreMe ignoreMe2 = IgnoreMe(const Target('8', 8, null)); + // ignore: prefer_const_constructors + final IgnoreMe ignoreMe3 = IgnoreMe(const Target('9', 9, Target('10', 10, null))); print(ignoreMe); + print(ignoreMe2); + print(ignoreMe3); } class IgnoreMe { From 0d6040d1c297e441db008f6f4fbe845e0a797285 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jan 2020 10:46:58 -0800 Subject: [PATCH 06/10] test pkg --- tools/const_finder/test/const_finder_test.dart | 18 ++++++++++++------ tools/const_finder/test/fixtures/.packages | 1 + .../const_finder/test/fixtures/lib/consts.dart | 4 ++++ .../test/fixtures/lib/consts_and_non.dart | 4 ++++ .../test/fixtures/pkg/package.dart | 12 ++++++++++++ 5 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 tools/const_finder/test/fixtures/pkg/package.dart diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index 9ac9e7c376f12..dac3a465a9ee2 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -50,6 +50,7 @@ void _checkConsts() { {'stringValue': '10', 'intValue': 10, 'targetValue': null}, {'stringValue': '9', 'intValue': 9}, {'stringValue': '7', 'intValue': 7, 'targetValue': null}, + {'stringValue': 'package', 'intValue':-1, 'targetValue': null}, ], 'nonConstantLocations': [], }), @@ -78,19 +79,24 @@ void _checkNonConsts() { 'nonConstantLocations': [ { 'file': 'file://$fixtures/lib/consts_and_non.dart', - 'line': 12, - 'column': 26 + 'line': 14, + 'column': 26, }, { 'file': 'file://$fixtures/lib/consts_and_non.dart', - 'line': 15, - 'column': 26 + 'line': 17, + 'column': 26, }, { 'file': 'file://$fixtures/lib/consts_and_non.dart', - 'line': 17, - 'column': 26 + 'line': 19, + 'column': 26, }, + { + 'file': 'file://$fixtures/pkg/package.dart', + 'line': 10, + 'column': 25, + } ] }), ); diff --git a/tools/const_finder/test/fixtures/.packages b/tools/const_finder/test/fixtures/.packages index 899ade66cd7b0..0ef0f0b0b3019 100644 --- a/tools/const_finder/test/fixtures/.packages +++ b/tools/const_finder/test/fixtures/.packages @@ -1,2 +1,3 @@ # Generated by pub on 2020-01-15 10:08:29.776333. const_finder_fixtures:lib/ +const_finder_fixtures_package:pkg/ \ No newline at end of file diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart index 63176e8dfe289..1fd3a64bd67cb 100644 --- a/tools/const_finder/test/fixtures/lib/consts.dart +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -4,6 +4,8 @@ import 'dart:core'; +import 'package:const_finder_fixtures_package/package.dart'; + import 'target.dart'; void main() { @@ -24,6 +26,8 @@ void main() { print(ignoreMe); print(ignoreMe2); print(ignoreMe3); + + createTargetInPackage(); } class IgnoreMe { diff --git a/tools/const_finder/test/fixtures/lib/consts_and_non.dart b/tools/const_finder/test/fixtures/lib/consts_and_non.dart index 9c832fa263170..9f4eeb8d04271 100644 --- a/tools/const_finder/test/fixtures/lib/consts_and_non.dart +++ b/tools/const_finder/test/fixtures/lib/consts_and_non.dart @@ -5,6 +5,8 @@ // ignore_for_file: prefer_const_constructors import 'dart:core'; +import 'package:const_finder_fixtures_package/package.dart'; + import 'target.dart'; void main() { @@ -28,6 +30,8 @@ void main() { print(ignoreMe); print(ignoreMe2); print(ignoreMe3); + + createNonConstTargetInPackage(); } class IgnoreMe { diff --git a/tools/const_finder/test/fixtures/pkg/package.dart b/tools/const_finder/test/fixtures/pkg/package.dart new file mode 100644 index 0000000000000..860f239be1f6d --- /dev/null +++ b/tools/const_finder/test/fixtures/pkg/package.dart @@ -0,0 +1,12 @@ +import 'package:const_finder_fixtures/target.dart'; + +void createTargetInPackage() { + const Target target = Target('package', -1, null); + target.hit(); +} + +void createNonConstTargetInPackage() { + // ignore: prefer_const_constructors + final Target target = Target('package_non', -2, null); + target.hit(); +} \ No newline at end of file From 340710ffdb2715243dc91a57607fc9de0bd0bfc6 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 22 Jan 2020 10:47:44 -0800 Subject: [PATCH 07/10] .. --- tools/const_finder/test/fixtures/pkg/package.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/const_finder/test/fixtures/pkg/package.dart b/tools/const_finder/test/fixtures/pkg/package.dart index 860f239be1f6d..62ba73c3575ab 100644 --- a/tools/const_finder/test/fixtures/pkg/package.dart +++ b/tools/const_finder/test/fixtures/pkg/package.dart @@ -9,4 +9,4 @@ void createNonConstTargetInPackage() { // ignore: prefer_const_constructors final Target target = Target('package_non', -2, null); target.hit(); -} \ No newline at end of file +} From 038221c078af29b0d034a194d45537159e45c506 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 2 Mar 2020 14:29:52 -0800 Subject: [PATCH 08/10] Fix const finder for static const map/list/set --- tools/const_finder/lib/const_finder.dart | 19 ++++++++++++- .../const_finder/test/const_finder_test.dart | 10 ++++++- .../test/fixtures/lib/consts.dart | 27 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart index 1eb8776a91f40..4605bb6c5298b 100644 --- a/tools/const_finder/lib/const_finder.dart +++ b/tools/const_finder/lib/const_finder.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:collection'; + import 'package:kernel/kernel.dart' hide MapEntry; import 'package:meta/meta.dart'; @@ -31,10 +33,25 @@ class _ConstVisitor extends RecursiveVisitor { final List> nonConstantLocations; bool _matches(Class node) { - return node.enclosingLibrary.canonicalName.name == classLibraryUri && + return node.enclosingLibrary.importUri.toString() == classLibraryUri && node.name == className; } + // Avoid visiting the same constant more than once. + Set _cache = LinkedHashSet.identity(); + + @override + void defaultConstant(Constant node) { + if (_cache.add(node)) { + super.defaultConstant(node); + } + } + + @override + void defaultConstantReference(Constant node) { + defaultConstant(node); + } + @override void visitConstructorInvocation(ConstructorInvocation node) { final Class parentClass = node.target.parent as Class; diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index dac3a465a9ee2..5631bd76b3658 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -37,11 +37,19 @@ void _checkConsts() { classLibraryUri: 'package:const_finder_fixtures/target.dart', className: 'Target', ); - expect( jsonEncode(finder.findInstances()), jsonEncode({ 'constantInstances': >[ + {'stringValue': '100', 'intValue': 100, 'targetValue': null}, + {'stringValue': '102', 'intValue': 102, 'targetValue': null}, + {'stringValue': '101', 'intValue': 101}, + {'stringValue': '103', 'intValue': 103, 'targetValue': null}, + {'stringValue': '105', 'intValue': 105, 'targetValue': null}, + {'stringValue': '104', 'intValue': 104}, + {'stringValue': '106', 'intValue': 106, 'targetValue': null}, + {'stringValue': '108', 'intValue': 108, 'targetValue': null}, + {'stringValue': '107', 'intValue': 107}, {'stringValue': '1', 'intValue': 1, 'targetValue': null}, {'stringValue': '4', 'intValue': 4, 'targetValue': null}, {'stringValue': '2', 'intValue': 2}, diff --git a/tools/const_finder/test/fixtures/lib/consts.dart b/tools/const_finder/test/fixtures/lib/consts.dart index 1fd3a64bd67cb..4095df150c94e 100644 --- a/tools/const_finder/test/fixtures/lib/consts.dart +++ b/tools/const_finder/test/fixtures/lib/consts.dart @@ -28,6 +28,9 @@ void main() { print(ignoreMe3); createTargetInPackage(); + + final StaticConstInitializer staticConstMap = StaticConstInitializer(); + staticConstMap.useOne(1); } class IgnoreMe { @@ -36,6 +39,30 @@ class IgnoreMe { final Target target; } +class StaticConstInitializer { + static const List targets = [ + Target('100', 100, null), + Target('101', 101, Target('102', 102, null)), + ]; + + static const Set targetSet = { + Target('103', 103, null), + Target('104', 104, Target('105', 105, null)), + }; + + + static const Map targetMap = { + 0: Target('106', 106, null), + 1: Target('107', 107, Target('108', 108, null)), + }; + + void useOne(int index) { + targets[index].hit(); + targetSet.skip(index).first.hit(); + targetMap[index].hit(); + } +} + void blah(Target target) { print(target); } From 76e00a2ea6438afad4c78cdf325aa5288e0d43ea Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 2 Mar 2020 15:45:46 -0800 Subject: [PATCH 09/10] review --- tools/const_finder/lib/const_finder.dart | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/const_finder/lib/const_finder.dart b/tools/const_finder/lib/const_finder.dart index 4605bb6c5298b..481bd24062c6a 100644 --- a/tools/const_finder/lib/const_finder.dart +++ b/tools/const_finder/lib/const_finder.dart @@ -15,7 +15,7 @@ class _ConstVisitor extends RecursiveVisitor { ) : assert(kernelFilePath != null), assert(classLibraryUri != null), assert(className != null), - _visitedInstnaces = {}, + _visitedInstances = {}, constantInstances = >[], nonConstantLocations = >[]; @@ -28,7 +28,7 @@ class _ConstVisitor extends RecursiveVisitor { /// The name of the class to find. final String className; - final Set _visitedInstnaces; + final Set _visitedInstances; final List> constantInstances; final List> nonConstantLocations; @@ -68,9 +68,8 @@ class _ConstVisitor extends RecursiveVisitor { @override void visitInstanceConstantReference(InstanceConstant node) { - node.fieldValues.values.whereType().forEach(visitInstanceConstantReference); + super.visitInstanceConstantReference(node); if (!_matches(node.classNode)) { - super.visitInstanceConstantReference(node); return; } final Map instance = {}; @@ -79,9 +78,9 @@ class _ConstVisitor extends RecursiveVisitor { continue; } final PrimitiveConstant value = kvp.value as PrimitiveConstant; - instance[kvp.key.canonicalName.name] = value.value; + instance[kvp.key.asField.name.name] = value.value; } - if (_visitedInstnaces.add(instance.toString())) { + if (_visitedInstances.add(instance.toString())) { constantInstances.add(instance); } } @@ -107,7 +106,7 @@ class ConstFinder { /// Finds all instances Map findInstances() { - _visitor._visitedInstnaces.clear(); + _visitor._visitedInstances.clear(); for (Library library in loadComponentFromBinary(_visitor.kernelFilePath).libraries) { library.visitChildren(_visitor); } From e2d498030f474407252b3b206373dfc333596838 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 2 Mar 2020 16:17:40 -0800 Subject: [PATCH 10/10] test for insane recursion --- .../const_finder/test/const_finder_test.dart | 30 ++- tools/const_finder/test/fixtures/lib/box.dart | 227 ++++++++++++++++++ 2 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 tools/const_finder/test/fixtures/lib/box.dart diff --git a/tools/const_finder/test/const_finder_test.dart b/tools/const_finder/test/const_finder_test.dart index 5631bd76b3658..c3b1a8dc20cbe 100644 --- a/tools/const_finder/test/const_finder_test.dart +++ b/tools/const_finder/test/const_finder_test.dart @@ -19,9 +19,11 @@ void expect(T value, T expected) { final String basePath = path.canonicalize(path.join(path.dirname(Platform.script.path), '..')); final String fixtures = path.join(basePath, 'test', 'fixtures'); +final String box = path.join(fixtures, 'lib', 'box.dart'); final String consts = path.join(fixtures, 'lib', 'consts.dart'); final String dotPackages = path.join(fixtures, '.packages'); final String constsAndNon = path.join(fixtures, 'lib', 'consts_and_non.dart'); +final String boxDill = path.join(fixtures, 'box.dill'); final String constsDill = path.join(fixtures, 'consts.dill'); final String constsAndNonDill = path.join(fixtures, 'consts_and_non.dill'); @@ -30,8 +32,19 @@ final String constsAndNonDill = path.join(fixtures, 'consts_and_non.dill'); final String dart = Platform.resolvedExecutable; final String bat = Platform.isWindows ? '.bat' : ''; +void _checkRecursion() { + stdout.writeln('Checking recursive calls.'); + final ConstFinder finder = ConstFinder( + kernelFilePath: boxDill, + classLibraryUri: 'package:const_finder_fixtures/box.dart', + className: 'Box', + ); + // Will timeout if we did things wrong. + jsonEncode(finder.findInstances()); +} + void _checkConsts() { - print('Checking for expected constants.'); + stdout.writeln('Checking for expected constants.'); final ConstFinder finder = ConstFinder( kernelFilePath: constsDill, classLibraryUri: 'package:const_finder_fixtures/target.dart', @@ -66,7 +79,7 @@ void _checkConsts() { } void _checkNonConsts() { - print('Checking for non-constant instances.'); + stdout.writeln('Checking for non-constant instances.'); final ConstFinder finder = ConstFinder( kernelFilePath: constsAndNonDill, classLibraryUri: 'package:const_finder_fixtures/target.dart', @@ -129,6 +142,18 @@ Future main(List args) async { stdout.writeln('Generating kernel fixtures...'); stdout.writeln(consts); + + _checkProcessResult(Process.runSync(dart, [ + frontendServer, + '--sdk-root=$sdkRoot', + '--target=flutter', + '--aot', + '--tfa', + '--packages=$dotPackages', + '--output-dill=$boxDill', + box, + ])); + _checkProcessResult(Process.runSync(dart, [ frontendServer, '--sdk-root=$sdkRoot', @@ -151,6 +176,7 @@ Future main(List args) async { constsAndNon, ])); + _checkRecursion(); _checkConsts(); _checkNonConsts(); } finally { diff --git a/tools/const_finder/test/fixtures/lib/box.dart b/tools/const_finder/test/fixtures/lib/box.dart new file mode 100644 index 0000000000000..f84961d10883b --- /dev/null +++ b/tools/const_finder/test/fixtures/lib/box.dart @@ -0,0 +1,227 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// If canonicalization uses deep structural hashing without memoizing, this +// will exhibit superlinear time. + +// Compare with Dart version of this test at: +// https://github.com/dart-lang/sdk/blob/ca3ad264a64937d5d336cd04dbf2746d1b7d8fc4/tests/language_2/canonicalize/hashing_memoize_instance_test.dart + +class Box { + final Object content1; + final Object content2; + const Box(this.content1, this.content2); +} + +const Box box1_0 = Box(null, null); +const Box box1_1 = Box(box1_0, box1_0); +const Box box1_2 = Box(box1_1, box1_1); +const Box box1_3 = Box(box1_2, box1_2); +const Box box1_4 = Box(box1_3, box1_3); +const Box box1_5 = Box(box1_4, box1_4); +const Box box1_6 = Box(box1_5, box1_5); +const Box box1_7 = Box(box1_6, box1_6); +const Box box1_8 = Box(box1_7, box1_7); +const Box box1_9 = Box(box1_8, box1_8); +const Box box1_10 = Box(box1_9, box1_9); +const Box box1_11 = Box(box1_10, box1_10); +const Box box1_12 = Box(box1_11, box1_11); +const Box box1_13 = Box(box1_12, box1_12); +const Box box1_14 = Box(box1_13, box1_13); +const Box box1_15 = Box(box1_14, box1_14); +const Box box1_16 = Box(box1_15, box1_15); +const Box box1_17 = Box(box1_16, box1_16); +const Box box1_18 = Box(box1_17, box1_17); +const Box box1_19 = Box(box1_18, box1_18); +const Box box1_20 = Box(box1_19, box1_19); +const Box box1_21 = Box(box1_20, box1_20); +const Box box1_22 = Box(box1_21, box1_21); +const Box box1_23 = Box(box1_22, box1_22); +const Box box1_24 = Box(box1_23, box1_23); +const Box box1_25 = Box(box1_24, box1_24); +const Box box1_26 = Box(box1_25, box1_25); +const Box box1_27 = Box(box1_26, box1_26); +const Box box1_28 = Box(box1_27, box1_27); +const Box box1_29 = Box(box1_28, box1_28); +const Box box1_30 = Box(box1_29, box1_29); +const Box box1_31 = Box(box1_30, box1_30); +const Box box1_32 = Box(box1_31, box1_31); +const Box box1_33 = Box(box1_32, box1_32); +const Box box1_34 = Box(box1_33, box1_33); +const Box box1_35 = Box(box1_34, box1_34); +const Box box1_36 = Box(box1_35, box1_35); +const Box box1_37 = Box(box1_36, box1_36); +const Box box1_38 = Box(box1_37, box1_37); +const Box box1_39 = Box(box1_38, box1_38); +const Box box1_40 = Box(box1_39, box1_39); +const Box box1_41 = Box(box1_40, box1_40); +const Box box1_42 = Box(box1_41, box1_41); +const Box box1_43 = Box(box1_42, box1_42); +const Box box1_44 = Box(box1_43, box1_43); +const Box box1_45 = Box(box1_44, box1_44); +const Box box1_46 = Box(box1_45, box1_45); +const Box box1_47 = Box(box1_46, box1_46); +const Box box1_48 = Box(box1_47, box1_47); +const Box box1_49 = Box(box1_48, box1_48); +const Box box1_50 = Box(box1_49, box1_49); +const Box box1_51 = Box(box1_50, box1_50); +const Box box1_52 = Box(box1_51, box1_51); +const Box box1_53 = Box(box1_52, box1_52); +const Box box1_54 = Box(box1_53, box1_53); +const Box box1_55 = Box(box1_54, box1_54); +const Box box1_56 = Box(box1_55, box1_55); +const Box box1_57 = Box(box1_56, box1_56); +const Box box1_58 = Box(box1_57, box1_57); +const Box box1_59 = Box(box1_58, box1_58); +const Box box1_60 = Box(box1_59, box1_59); +const Box box1_61 = Box(box1_60, box1_60); +const Box box1_62 = Box(box1_61, box1_61); +const Box box1_63 = Box(box1_62, box1_62); +const Box box1_64 = Box(box1_63, box1_63); +const Box box1_65 = Box(box1_64, box1_64); +const Box box1_66 = Box(box1_65, box1_65); +const Box box1_67 = Box(box1_66, box1_66); +const Box box1_68 = Box(box1_67, box1_67); +const Box box1_69 = Box(box1_68, box1_68); +const Box box1_70 = Box(box1_69, box1_69); +const Box box1_71 = Box(box1_70, box1_70); +const Box box1_72 = Box(box1_71, box1_71); +const Box box1_73 = Box(box1_72, box1_72); +const Box box1_74 = Box(box1_73, box1_73); +const Box box1_75 = Box(box1_74, box1_74); +const Box box1_76 = Box(box1_75, box1_75); +const Box box1_77 = Box(box1_76, box1_76); +const Box box1_78 = Box(box1_77, box1_77); +const Box box1_79 = Box(box1_78, box1_78); +const Box box1_80 = Box(box1_79, box1_79); +const Box box1_81 = Box(box1_80, box1_80); +const Box box1_82 = Box(box1_81, box1_81); +const Box box1_83 = Box(box1_82, box1_82); +const Box box1_84 = Box(box1_83, box1_83); +const Box box1_85 = Box(box1_84, box1_84); +const Box box1_86 = Box(box1_85, box1_85); +const Box box1_87 = Box(box1_86, box1_86); +const Box box1_88 = Box(box1_87, box1_87); +const Box box1_89 = Box(box1_88, box1_88); +const Box box1_90 = Box(box1_89, box1_89); +const Box box1_91 = Box(box1_90, box1_90); +const Box box1_92 = Box(box1_91, box1_91); +const Box box1_93 = Box(box1_92, box1_92); +const Box box1_94 = Box(box1_93, box1_93); +const Box box1_95 = Box(box1_94, box1_94); +const Box box1_96 = Box(box1_95, box1_95); +const Box box1_97 = Box(box1_96, box1_96); +const Box box1_98 = Box(box1_97, box1_97); +const Box box1_99 = Box(box1_98, box1_98); + +const Box box2_0 = Box(null, null); +const Box box2_1 = Box(box2_0, box2_0); +const Box box2_2 = Box(box2_1, box2_1); +const Box box2_3 = Box(box2_2, box2_2); +const Box box2_4 = Box(box2_3, box2_3); +const Box box2_5 = Box(box2_4, box2_4); +const Box box2_6 = Box(box2_5, box2_5); +const Box box2_7 = Box(box2_6, box2_6); +const Box box2_8 = Box(box2_7, box2_7); +const Box box2_9 = Box(box2_8, box2_8); +const Box box2_10 = Box(box2_9, box2_9); +const Box box2_11 = Box(box2_10, box2_10); +const Box box2_12 = Box(box2_11, box2_11); +const Box box2_13 = Box(box2_12, box2_12); +const Box box2_14 = Box(box2_13, box2_13); +const Box box2_15 = Box(box2_14, box2_14); +const Box box2_16 = Box(box2_15, box2_15); +const Box box2_17 = Box(box2_16, box2_16); +const Box box2_18 = Box(box2_17, box2_17); +const Box box2_19 = Box(box2_18, box2_18); +const Box box2_20 = Box(box2_19, box2_19); +const Box box2_21 = Box(box2_20, box2_20); +const Box box2_22 = Box(box2_21, box2_21); +const Box box2_23 = Box(box2_22, box2_22); +const Box box2_24 = Box(box2_23, box2_23); +const Box box2_25 = Box(box2_24, box2_24); +const Box box2_26 = Box(box2_25, box2_25); +const Box box2_27 = Box(box2_26, box2_26); +const Box box2_28 = Box(box2_27, box2_27); +const Box box2_29 = Box(box2_28, box2_28); +const Box box2_30 = Box(box2_29, box2_29); +const Box box2_31 = Box(box2_30, box2_30); +const Box box2_32 = Box(box2_31, box2_31); +const Box box2_33 = Box(box2_32, box2_32); +const Box box2_34 = Box(box2_33, box2_33); +const Box box2_35 = Box(box2_34, box2_34); +const Box box2_36 = Box(box2_35, box2_35); +const Box box2_37 = Box(box2_36, box2_36); +const Box box2_38 = Box(box2_37, box2_37); +const Box box2_39 = Box(box2_38, box2_38); +const Box box2_40 = Box(box2_39, box2_39); +const Box box2_41 = Box(box2_40, box2_40); +const Box box2_42 = Box(box2_41, box2_41); +const Box box2_43 = Box(box2_42, box2_42); +const Box box2_44 = Box(box2_43, box2_43); +const Box box2_45 = Box(box2_44, box2_44); +const Box box2_46 = Box(box2_45, box2_45); +const Box box2_47 = Box(box2_46, box2_46); +const Box box2_48 = Box(box2_47, box2_47); +const Box box2_49 = Box(box2_48, box2_48); +const Box box2_50 = Box(box2_49, box2_49); +const Box box2_51 = Box(box2_50, box2_50); +const Box box2_52 = Box(box2_51, box2_51); +const Box box2_53 = Box(box2_52, box2_52); +const Box box2_54 = Box(box2_53, box2_53); +const Box box2_55 = Box(box2_54, box2_54); +const Box box2_56 = Box(box2_55, box2_55); +const Box box2_57 = Box(box2_56, box2_56); +const Box box2_58 = Box(box2_57, box2_57); +const Box box2_59 = Box(box2_58, box2_58); +const Box box2_60 = Box(box2_59, box2_59); +const Box box2_61 = Box(box2_60, box2_60); +const Box box2_62 = Box(box2_61, box2_61); +const Box box2_63 = Box(box2_62, box2_62); +const Box box2_64 = Box(box2_63, box2_63); +const Box box2_65 = Box(box2_64, box2_64); +const Box box2_66 = Box(box2_65, box2_65); +const Box box2_67 = Box(box2_66, box2_66); +const Box box2_68 = Box(box2_67, box2_67); +const Box box2_69 = Box(box2_68, box2_68); +const Box box2_70 = Box(box2_69, box2_69); +const Box box2_71 = Box(box2_70, box2_70); +const Box box2_72 = Box(box2_71, box2_71); +const Box box2_73 = Box(box2_72, box2_72); +const Box box2_74 = Box(box2_73, box2_73); +const Box box2_75 = Box(box2_74, box2_74); +const Box box2_76 = Box(box2_75, box2_75); +const Box box2_77 = Box(box2_76, box2_76); +const Box box2_78 = Box(box2_77, box2_77); +const Box box2_79 = Box(box2_78, box2_78); +const Box box2_80 = Box(box2_79, box2_79); +const Box box2_81 = Box(box2_80, box2_80); +const Box box2_82 = Box(box2_81, box2_81); +const Box box2_83 = Box(box2_82, box2_82); +const Box box2_84 = Box(box2_83, box2_83); +const Box box2_85 = Box(box2_84, box2_84); +const Box box2_86 = Box(box2_85, box2_85); +const Box box2_87 = Box(box2_86, box2_86); +const Box box2_88 = Box(box2_87, box2_87); +const Box box2_89 = Box(box2_88, box2_88); +const Box box2_90 = Box(box2_89, box2_89); +const Box box2_91 = Box(box2_90, box2_90); +const Box box2_92 = Box(box2_91, box2_91); +const Box box2_93 = Box(box2_92, box2_92); +const Box box2_94 = Box(box2_93, box2_93); +const Box box2_95 = Box(box2_94, box2_94); +const Box box2_96 = Box(box2_95, box2_95); +const Box box2_97 = Box(box2_96, box2_96); +const Box box2_98 = Box(box2_97, box2_97); +const Box box2_99 = Box(box2_98, box2_98); + +Object confuse(Box x) { + try { throw x; } catch (e) { return e; } +} + +void main() { + if (!identical(confuse(box1_99), confuse(box2_99))) { + throw Exception('box1_99 !== box2_99'); + } +}