[dart2wasm] Fix record representation equality in dynamic modules.

Generated Record `==` methods begin by doing a type check on the argument. Today this is implemented as an interface subtype check like `other is <ThisRecordType>`. This generally ends up being a class ID check. For dynamic modules, the class ID is not statically known so the type test fails.

Instead for dynamic modules we can have the `==` do a full check against the RTI object using `_checkInstance`. This will correctly check the structure of the type rather than just the class ID.

TEST=N/A small signature change.

Change-Id: I86832d2bcdf0e95bd85267c016e5e69b002bcb68
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416323
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>

Author: natebiggs

pkg/dart2wasm/lib/compile.dart

@@ -244,8 +244,8 @@ Future<CompilationResult> compileToModule(
       ? null
       : js.createRuntimeFinalizer(component, coreTypes, classHierarchy);
 
-  final Map<RecordShape, Class> recordClasses =
-      generateRecordClasses(component, coreTypes);
+  final Map<RecordShape, Class> recordClasses = generateRecordClasses(
+      component, coreTypes, isDynamicMainModule || isDynamicModule);
   target.recordClasses = recordClasses;
 
   if (options.dumpKernelBeforeTfa != null) {

pkg/dart2wasm/lib/dynamic_modules.dart

@@ -183,6 +183,9 @@ class DynamicMainModuleStrategy extends DefaultModuleStrategy with KernelNodes {
       add(procedure, kDynModuleCallablePragmaName);
     }
 
+    // Mark all record classes as dynamic module extendable.
+    addPragma(coreTypes.recordClass, kDynModuleExtendablePragmaName, coreTypes);
+
     // SystemHash.combine used by closures.
     add(systemHashCombine, kDynModuleCallablePragmaName);
   }
@@ -361,6 +364,7 @@ void _recordIdDynamic(w.FunctionBuilder f, Translator translator) {
     ib.i32_const(0);
   } else {
     ib.local_get(ib.locals[0]);
+    translator.callReference(translator.localizeClassId.reference, ib);
     ib.emitClassIdRangeCheck(ranges);
   }
   ib.end();

pkg/dart2wasm/lib/record_class_generator.dart

@@ -81,9 +81,10 @@ import 'util.dart';
 /// }
 /// ```
 Map<RecordShape, Class> generateRecordClasses(
-    Component component, CoreTypes coreTypes) {
+    Component component, CoreTypes coreTypes, bool isDynamicModuleEnabled) {
   final Map<RecordShape, Class> recordClasses = {};
-  final recordClassGenerator = _RecordClassGenerator(recordClasses, coreTypes);
+  final recordClassGenerator =
+      _RecordClassGenerator(recordClasses, coreTypes, isDynamicModuleEnabled);
   final visitor = _RecordVisitor(recordClassGenerator);
   component.libraries.forEach(visitor.visitLibrary);
   return recordClasses;
@@ -92,6 +93,7 @@ Map<RecordShape, Class> generateRecordClasses(
 class _RecordClassGenerator {
   final CoreTypes coreTypes;
   final Map<RecordShape, Class> classes;
+  final bool isDynamicModuleEnabled;
 
   late final Class typeRuntimetypeTypeClass =
       coreTypes.index.getClass("dart:core", "_Type");
@@ -190,7 +192,8 @@ class _RecordClassGenerator {
     return map;
   })();
 
-  _RecordClassGenerator(this.classes, this.coreTypes);
+  _RecordClassGenerator(
+      this.classes, this.coreTypes, this.isDynamicModuleEnabled);
 
   void generateClassForRecordType(RecordType recordType) {
     final shape = RecordShape.fromType(recordType);
@@ -228,9 +231,10 @@ class _RecordClassGenerator {
         coreTypes);
 
     library.addClass(cls);
-    cls.addProcedure(_generateEquals(shape, fields, cls));
+    final getRti = _generateRecordRuntimeType(shape, fields);
+    cls.addProcedure(_generateEquals(shape, fields, cls, getRti));
     cls.addProcedure(_generateCheckRecordType(shape, fields));
-    cls.addProcedure(_generateRecordRuntimeType(shape, fields));
+    cls.addProcedure(getRti);
     cls.addProcedure(_generateMasqueradedRecordRuntimeType(shape, fields));
     return cls;
   }
@@ -385,7 +389,8 @@ class _RecordClassGenerator {
   }
 
   /// Generate `bool operator ==` member.
-  Procedure _generateEquals(RecordShape shape, List<Field> fields, Class cls) {
+  Procedure _generateEquals(
+      RecordShape shape, List<Field> fields, Class cls, Procedure getRti) {
     final equalsFunctionType = FunctionType(
       [nullableObjectType],
       boolType,
@@ -397,12 +402,32 @@ class _RecordClassGenerator {
 
     final List<Statement> statements = [];
 
-    statements.add(IfStatement(
-      Not(IsExpression(
-          VariableGet(parameter), InterfaceType(cls, Nullability.nonNullable))),
-      ReturnStatement(BoolLiteral(false)),
-      null,
-    ));
+    if (isDynamicModuleEnabled) {
+      final checkInstance = coreTypes.index
+          .getProcedure('dart:core', '_RecordType', '_checkInstance');
+      statements.add(IfStatement(
+        Not(InstanceInvocation(
+          InstanceAccessKind.Instance,
+          (InstanceInvocation(InstanceAccessKind.Instance, ThisExpression(),
+              getRti.name, Arguments([]),
+              interfaceTarget: getRti,
+              functionType: getRti.computeSignatureOrFunctionType())),
+          checkInstance.name,
+          Arguments([VariableGet(parameter)]),
+          interfaceTarget: checkInstance,
+          functionType: checkInstance.computeSignatureOrFunctionType(),
+        )),
+        ReturnStatement(BoolLiteral(false)),
+        null,
+      ));
+    } else {
+      statements.add(IfStatement(
+        Not(IsExpression(VariableGet(parameter),
+            InterfaceType(cls, Nullability.nonNullable))),
+        ReturnStatement(BoolLiteral(false)),
+        null,
+      ));
+    }
 
     // Compare fields.
     for (Field field in fields) {

pkg/vm/test/transformations/type_flow/transformer_test.dart

@@ -50,7 +50,7 @@ void runTestCase(
   if (target is WasmTarget) {
     // Keep these flags in-sync with pkg/dart2wasm/lib/compile.dart
     useRapidTypeAnalysis = false;
-    target.recordClasses = generateRecordClasses(component, coreTypes);
+    target.recordClasses = generateRecordClasses(component, coreTypes, false);
   }
 
   component = transformComponent(

sdk/lib/_internal/wasm/lib/record_patch.dart

@@ -12,6 +12,7 @@ abstract class Record {
   _RecordType get _masqueradedRecordRuntimeType;
   _RecordType get _recordRuntimeType;
 
+  @pragma('dyn-module:can-be-overridden')
   bool _checkRecordType(
     WasmArray<_Type> types,
     ImmutableWasmArray<String> names,

sdk/lib/_internal/wasm/lib/type.dart

@@ -580,6 +580,7 @@ class _RecordType extends _Type {
   _Type get _asNullable => _RecordType(names, fieldTypes, true);
 
   @override
+  @pragma('dyn-module:callable')
   bool _checkInstance(Object o) {
     if (!_isRecordClassId(ClassID.getID(o))) return false;
     return unsafeCast<Record>(o)._checkRecordType(fieldTypes, names);
@@ -1932,6 +1933,7 @@ _Type _getActualRuntimeType(Object object) {
 }
 
 @pragma("wasm:prefer-inline")
+@pragma('dyn-module:callable')
 _Type _getActualRuntimeTypeNullable(Object? object) =>
     object == null ? _literal<Null>() : _getActualRuntimeType(object);