Add recording of constant instances to record_use

TESTED=pkg/vm/test/transformations/record_use_test.dart

Change-Id: I73ee5b7f1478286252664c53397270906ead549b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384280
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Moritz Sümmermann <[email protected]>

pkg/dartdev/test/native_assets/build_test.dart

@@ -234,77 +234,57 @@ void main(List<String> args) {
     });
   });
 
-  test('Tree-shaking: No assets are dropped', timeout: longTimeout, () async {
-    await recordUseTest('drop_dylib_recording', (dartAppUri) async {
-      // First try using all symbols, so no assets are treeshaken.
-      await runDart(
-        arguments: [
-          '--enable-experiment=native-assets,record-use',
-          'build',
-          'bin/drop_dylib_recording_all.dart',
-        ],
-        workingDirectory: dartAppUri,
-        logger: logger,
-        expectExitCodeZero: true,
-      );
+  for (var filename in [
+    'drop_dylib_recording_calls',
+    'drop_dylib_recording_instances',
+  ]) {
+    test('Tree-shaking in $filename: An asset is dropped', timeout: longTimeout,
+        () async {
+      await recordUseTest('drop_dylib_recording', (dartAppUri) async {
+        final addLib =
+            OSImpl.current.libraryFileName('add', DynamicLoadingBundledImpl());
+        final mulitplyLib = OSImpl.current
+            .libraryFileName('multiply', DynamicLoadingBundledImpl());
+        // Now try using the add symbol only, so the multiply library is
+        // tree-shaken.
 
-      // The build directory exists
-      final allDirectory =
-          Directory.fromUri(dartAppUri.resolve('bin/drop_dylib_recording_all'));
-      expect(allDirectory.existsSync(), true);
+        await runDart(
+          arguments: [
+            '--enable-experiment=native-assets,record-use',
+            'build',
+            'bin/$filename.dart',
+          ],
+          workingDirectory: dartAppUri,
+          logger: logger,
+          expectExitCodeZero: true,
+        );
 
-      // No assets have been treeshaken
-      final addLib =
-          OSImpl.current.libraryFileName('add', DynamicLoadingBundledImpl());
-      final mulitplyLib = OSImpl.current
-          .libraryFileName('multiply', DynamicLoadingBundledImpl());
-      expect(
-        File.fromUri(allDirectory.uri.resolve('lib/$addLib')).existsSync(),
-        true,
-      );
-      expect(
-        File.fromUri(allDirectory.uri.resolve('lib/$mulitplyLib')).existsSync(),
-        true,
-      );
-    });
-  });
+        await runProcess(
+          executable: Uri.file('bin/$filename/$filename.exe'),
+          logger: logger,
+          expectedExitCode: 0,
+          throwOnUnexpectedExitCode: true,
+          workingDirectory: dartAppUri,
+        );
 
-  test('Tree-shaking: An asset is dropped', timeout: longTimeout, () async {
-    await recordUseTest('drop_dylib_recording', (dartAppUri) async {
-      final addLib =
-          OSImpl.current.libraryFileName('add', DynamicLoadingBundledImpl());
-      final mulitplyLib = OSImpl.current
-          .libraryFileName('multiply', DynamicLoadingBundledImpl());
-      // Now try using the add symbol only, so the multiply library is
-      // tree-shaken.
-      await runDart(
-        arguments: [
-          '--enable-experiment=native-assets,record-use',
-          'build',
-          'bin/drop_dylib_recording_shake.dart',
-        ],
-        workingDirectory: dartAppUri,
-        logger: logger,
-        expectExitCodeZero: true,
-      );
+        // The build directory exists
+        final shakeDirectory =
+            Directory.fromUri(dartAppUri.resolve('bin/$filename'));
+        expect(shakeDirectory.existsSync(), true);
 
-      // The build directory exists
-      final shakeDirectory = Directory.fromUri(
-          dartAppUri.resolve('bin/drop_dylib_recording_shake'));
-      expect(shakeDirectory.existsSync(), true);
-
-      // The multiply asset has been treeshaken
-      expect(
-        File.fromUri(shakeDirectory.uri.resolve('lib/$addLib')).existsSync(),
-        true,
-      );
-      expect(
-        File.fromUri(shakeDirectory.uri.resolve('lib/$mulitplyLib'))
-            .existsSync(),
-        false,
-      );
+        // The multiply asset has been treeshaken
+        expect(
+          File.fromUri(shakeDirectory.uri.resolve('lib/$addLib')).existsSync(),
+          true,
+        );
+        expect(
+          File.fromUri(shakeDirectory.uri.resolve('lib/$mulitplyLib'))
+              .existsSync(),
+          false,
+        );
+      });
     });
-  });
+  }
 }
 
 Future<void> _withTempDir(Future<void> Function(Uri tempUri) fun) async {

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

@@ -25,22 +25,37 @@ Iterable<InstanceConstant> findRecordUseAnnotation(Annotatable node) =>
 // Coverage-ignore(suite): Not run.
 final Uri _metaLibraryUri = new Uri(scheme: 'package', path: 'meta/meta.dart');
 
-bool isRecordUse(Class classNode) =>
-    classNode.name == 'RecordUse' &&
+// Coverage-ignore(suite): Not run.
+bool isRecordUse(Class cls) =>
+    cls.name == 'RecordUse' &&
     // Coverage-ignore(suite): Not run.
-    classNode.enclosingLibrary.importUri == _metaLibraryUri;
+    cls.enclosingLibrary.importUri == _metaLibraryUri;
+
+// Coverage-ignore(suite): Not run.
+bool isBeingRecorded(Class cls) => isRecordUse(cls) || hasRecordUse(cls);
+
+/// If [cls] annotation is in turn annotated by a recording annotation.
+// Coverage-ignore(suite): Not run.
+bool hasRecordUse(Class cls) => cls.annotations
+    .whereType<ConstantExpression>()
+    .map((e) => e.constant)
+    .whereType<InstanceConstant>()
+    .any((annotation) => isRecordUse(annotation.classNode));
 
 // Coverage-ignore(suite): Not run.
 /// Report if the resource annotations is placed on anything but a static
-/// method.
+/// method or a class without a const constructor.
 void validateRecordUseDeclaration(
   Annotatable node,
   ErrorReporter errorReporter,
   Iterable<InstanceConstant> resourceAnnotations,
 ) {
   final bool onNonStaticMethod =
       node is! Procedure || !node.isStatic || node.kind != ProcedureKind.Method;
-  if (onNonStaticMethod) {
+
+  final bool onClassWithoutConstConstructor = node is! Class ||
+      !node.constructors.any((constructor) => constructor.isConst);
+  if (onNonStaticMethod && onClassWithoutConstConstructor) {
     errorReporter.report(messageRecordUseCannotBePlacedHere.withLocation(
         node.location!.file, node.fileOffset, 1));
   }

pkg/record_use/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.3.0
+
+- Make `InstanceConstant` a `Constant`.
+- Separate import from location uri.
+
 ## 0.2.0
 
 - Use maps instead of lists in serialization.

pkg/record_use/lib/record_use.dart

@@ -8,14 +8,14 @@ export 'src/public/constant.dart'
     show
         BoolConstant,
         Constant,
+        InstanceConstant,
         IntConstant,
         ListConstant,
         MapConstant,
         NullConstant,
         PrimitiveConstant,
         StringConstant;
 export 'src/public/identifier.dart' show Identifier;
-export 'src/public/instance_constant.dart' show InstanceConstant;
 export 'src/public/location.dart' show Location;
 export 'src/public/metadata.dart' show Metadata;
 //Not exporting `Reference` as it is not used in the API

pkg/record_use/lib/src/internal/definition.dart

@@ -21,15 +21,12 @@ class Definition {
   factory Definition.fromJson(
     Map<String, dynamic> json,
     List<Identifier> identifiers,
+    List<String> uris,
   ) {
     final identifier = identifiers[json['id'] as int];
     return Definition(
       identifier: identifier,
-      location: Location.fromJson(
-        json['@'] as Map<String, dynamic>,
-        identifier.uri,
-        null,
-      ),
+      location: Location.fromJson(json['@'] as Map<String, dynamic>, uris),
       loadingUnit: json['loadingUnit'] as String?,
     );
   }
@@ -40,7 +37,7 @@ class Definition {
   ) =>
       {
         'id': identifiers[identifier]!,
-        '@': location.toJson(),
+        '@': location.toJson(uris),
         'loadingUnit': loadingUnit,
       };
 

pkg/record_use/lib/src/internal/usage.dart

@@ -28,6 +28,7 @@ class Usage<T extends Reference> {
         definition: Definition.fromJson(
           json['definition'] as Map<String, dynamic>,
           identifiers,
+          uris,
         ),
         references: (json['references'] as List)
             .map((x) => constr(x as Map<String, dynamic>, uris, constants))

pkg/record_use/lib/src/internal/usage_record.dart

@@ -71,7 +71,7 @@ class UsageRecord {
     }.asMapToIndices;
 
     final uris = <String>{
-      ...identifiers.keys.map((e) => e.uri),
+      ...identifiers.keys.map((e) => e.importUri),
       ...calls.expand((call) => [
             call.definition.location.uri,
             ...call.references.map((reference) => reference.location.uri),
@@ -87,9 +87,10 @@ class UsageRecord {
           .map((e) => e.arguments?.constArguments)
           .whereType<ConstArguments>()
           .expand((e) => {...e.named.values, ...e.positional.values})),
-      ...instances
-          .expand((element) => element.references)
-          .expand((e) => e.instanceConstant.fields.values)
+      ...instances.expand((element) => element.references).expand((e) => {
+            ...e.instanceConstant.fields.values,
+            e.instanceConstant,
+          })
     }.flatten().asMapToIndices;
     return {
       'metadata': metadata.toJson(),

pkg/record_use/lib/src/public/constant.dart

@@ -23,6 +23,9 @@ sealed class Constant {
         MapConstant._type => MapConstant(
             (value['value'] as Map<String, dynamic>)
                 .map((key, value) => MapEntry(key, constants[value as int]))),
+        InstanceConstant._type => InstanceConstant(
+            fields: (value['value'] as Map<String, dynamic>)
+                .map((key, value) => MapEntry(key, constants[value as int]))),
         String() =>
           throw UnimplementedError('This type is not a supported constant'),
       };
@@ -134,6 +137,45 @@ final class MapConstant<T extends Constant> extends Constant {
       );
 }
 
+final class InstanceConstant extends Constant {
+  static const _type = 'Instance';
+
+  final Map<String, Constant> fields;
+
+  const InstanceConstant({
+    required this.fields,
+  });
+
+  factory InstanceConstant.fromJson(
+    Map<String, dynamic> json,
+    List<Constant> constants,
+  ) {
+    return InstanceConstant(
+      fields: json.map((key, constantIndex) =>
+          MapEntry(key, constants[constantIndex as int])),
+    );
+  }
+
+  @override
+  Map<String, dynamic> toJson(Map<Constant, int> constants) => _toJson(
+        _type,
+        fields.isNotEmpty
+            ? fields.map((name, constantIndex) =>
+                MapEntry(name, constants[constantIndex]!))
+            : null,
+      );
+
+  @override
+  bool operator ==(Object other) {
+    if (identical(this, other)) return true;
+
+    return other is InstanceConstant && deepEquals(other.fields, fields);
+  }
+
+  @override
+  int get hashCode => deepHash(fields);
+}
+
 Map<String, dynamic> _toJson(String type, Object? value) {
   return {
     'type': type,

pkg/record_use/lib/src/public/identifier.dart

@@ -3,25 +3,25 @@
 // BSD-style license that can be found in the LICENSE file.
 
 class Identifier {
-  final String uri;
+  final String importUri;
   final String? parent; // Optional since not all elements have parents
   final String name;
 
   const Identifier({
-    required this.uri,
+    required this.importUri,
     this.parent,
     required this.name,
   });
 
   factory Identifier.fromJson(Map<String, dynamic> json, List<String> uris) =>
       Identifier(
-        uri: uris[json['uri'] as int],
+        importUri: uris[json['uri'] as int],
         parent: json['parent'] as String?,
         name: json['name'] as String,
       );
 
   Map<String, dynamic> toJson(Map<String, int> uris) => {
-        'uri': uris[uri]!,
+        'uri': uris[importUri]!,
         if (parent != null) 'parent': parent,
         'name': name,
       };
@@ -31,11 +31,11 @@ class Identifier {
     if (identical(this, other)) return true;
 
     return other is Identifier &&
-        other.uri == uri &&
+        other.importUri == importUri &&
         other.parent == parent &&
         other.name == name;
   }
 
   @override
-  int get hashCode => Object.hash(uri, parent, name);
+  int get hashCode => Object.hash(importUri, parent, name);
 }