[vm] Relax assertion about duplicated direct implementors

When mixin application is transformed, mixin is added to the end of
interfaces list. This may cause assertion failure if the same class
is used as mixin and mentioned among implemented interfaces.
This CL relaxes that assertion for mixins as this duplication is very
rare and seems harmless.

Note that it would not be valid to remove duplicated interface/mixin
from the list of interfaces, as it would break dart:mirrors in this case:
dart:mirrors needs to reflect duplicated interface/mixin both in
ClassMirror.superinterfaces (which is populated from the
list of interfaces without last element) and ClassMirror.mixin (the last
element in the list of interfaces).

Fixes #35854

Change-Id: I9b986f7759a76cb6f963ebd837e9ea26f6242f8e
Reviewed-on: https://dart-review.googlesource.com/c/91961
Reviewed-by: Régis Crelier <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

runtime/vm/class_finalizer.cc

@@ -1078,11 +1078,15 @@ void ClassFinalizer::FinalizeTypesInClass(const Class& cls) {
   // classes.
   Zone* zone = thread->zone();
   auto& interface_class = Class::Handle(zone);
+  const intptr_t mixin_index = cls.is_transformed_mixin_application()
+                                   ? interface_types.Length() - 1
+                                   : -1;
   for (intptr_t i = 0; i < interface_types.Length(); ++i) {
     interface_type ^= interface_types.At(i);
     interface_class = interface_type.type_class();
     MarkImplemented(thread->zone(), interface_class);
-    interface_class.AddDirectImplementor(cls);
+    interface_class.AddDirectImplementor(cls,
+                                         /* is_mixin = */ i == mixin_index);
   }
 
   if (FLAG_use_cha_deopt) {

runtime/vm/isolate_reload.cc

@@ -2269,10 +2269,14 @@ void IsolateReloadContext::RebuildDirectSubclasses() {
 
       interface_types = cls.interfaces();
       if (!interface_types.IsNull()) {
+        const intptr_t mixin_index = cls.is_transformed_mixin_application()
+                                         ? interface_types.Length() - 1
+                                         : -1;
         for (intptr_t j = 0; j < interface_types.Length(); ++j) {
           interface_type ^= interface_types.At(j);
           interface_class = interface_type.type_class();
-          interface_class.AddDirectImplementor(cls);
+          interface_class.AddDirectImplementor(
+              cls, /* is_mixin = */ i == mixin_index);
         }
       }
     }

runtime/vm/object.cc

@@ -3979,7 +3979,8 @@ RawClass* Class::GetPatchClass() const {
   return lib.GetPatchClass(String::Handle(Name()));
 }
 
-void Class::AddDirectImplementor(const Class& implementor) const {
+void Class::AddDirectImplementor(const Class& implementor,
+                                 bool is_mixin) const {
   ASSERT(is_implemented());
   ASSERT(!implementor.IsNull());
   GrowableObjectArray& direct_implementors =
@@ -3990,8 +3991,14 @@ void Class::AddDirectImplementor(const Class& implementor) const {
   }
 #if defined(DEBUG)
   // Verify that the same class is not added twice.
-  for (intptr_t i = 0; i < direct_implementors.Length(); i++) {
-    ASSERT(direct_implementors.At(i) != implementor.raw());
+  // The only exception is mixins: when mixin application is transformed,
+  // mixin is added to the end of interfaces list and may be duplicated:
+  //   class X = A with B implements B;
+  // This is rare and harmless.
+  if (!is_mixin) {
+    for (intptr_t i = 0; i < direct_implementors.Length(); i++) {
+      ASSERT(direct_implementors.At(i) != implementor.raw());
+    }
   }
 #endif
   direct_implementors.Add(implementor, Heap::kOld);

runtime/vm/object.h

@@ -962,7 +962,7 @@ class Class : public Object {
   RawGrowableObjectArray* direct_implementors() const {
     return raw_ptr()->direct_implementors_;
   }
-  void AddDirectImplementor(const Class& subclass) const;
+  void AddDirectImplementor(const Class& subclass, bool is_mixin) const;
   void ClearDirectImplementors() const;
 
   // Returns the list of classes having this class as direct superclass.

tests/co19_2/co19_2-kernel.status

@@ -751,9 +751,6 @@ Language/Statements/Switch/equal_operator_t02: MissingCompileTimeError # Issue 3
 LibTest/io/Stdin/readByteSync_A02_t01: RuntimeError, Pass
 LibTest/io/WebSocket/pingInterval_A01_t01: RuntimeError, Pass
 
-[ $mode == debug && $runtime == vm && ($compiler == dartk || $compiler == dartkb) ]
-Language/Classes/definition_t23: Crash # Issue 35854
-
 [ $runtime == vm && $system == linux && ($compiler == dartk || $compiler == dartkb) ]
 LibTest/io/Link/stat_A01_t01: RuntimeError
 LibTest/isolate/Isolate/spawn_A06_t03: Crash, Pass