Adjusted how dtors get called on types that don't support non-destructive moves

distr/flecs.c

@@ -41235,11 +41235,12 @@ void flecs_table_delete(
 
                 /* If neither move nor move_ctor are set, this indicates that 
                  * non-destructive move semantics are not supported for this 
-                 * type. In such cases, we set the move_dtor as ctor_move_dtor, 
-                 * which indicates a destructive move operation. This adjustment 
-                 * ensures compatibility with different language bindings. */
-                if (!ti->hooks.move_ctor && ti->hooks.ctor_move_dtor) {
-                  move_dtor = ti->hooks.ctor_move_dtor;
+                 * type. In such cases when we are performing a non-destructive deletion, 
+                 * we set the move_dtor as ctor_move_dtor, which indicates we are operating
+                 * on memory already cleaned up. This adjustment ensures compatibility
+                 * with different language bindings. */
+                if (!destruct && !ti->hooks.move_ctor) {
+                    move_dtor = ti->hooks.ctor_move_dtor;
                 }
 
                 if (move_dtor) {
@@ -41376,8 +41377,14 @@ void flecs_table_move(
                 flecs_table_invoke_add_hooks(world, dst_table,
                     i_new, &dst_entity, dst_index, 1, construct);
             } else {
+                ecs_type_info_t *ti = dst_column->ti;
+
+                /* If the component doesn't have a move/move_ctor,
+                 * then it doesn't support non-destructive moves and must
+                 * be destructed during its removal.
+                 */
                 flecs_table_invoke_remove_hooks(world, src_table,
-                    src_column, &src_entity, src_index, 1, use_move_dtor);
+                    src_column, &src_entity, src_index, 1, use_move_dtor || !ti->hooks.move_ctor);
             }
         }
 
@@ -41391,8 +41398,11 @@ void flecs_table_move(
     }
 
     for (; (i_old < src_column_count); i_old ++) {
+        ecs_column_t *src_column = &src_columns[i_new];
+        ecs_type_info_t *ti = src_column->ti;
+
         flecs_table_invoke_remove_hooks(world, src_table, &src_columns[i_old], 
-            &src_entity, src_index, 1, use_move_dtor);
+            &src_entity, src_index, 1, use_move_dtor || !ti->hooks.move_ctor);
     }
 
     flecs_table_check_sanity(dst_table);

src/storage/table.c

@@ -1845,11 +1845,12 @@ void flecs_table_delete(
 
                 /* If neither move nor move_ctor are set, this indicates that 
                  * non-destructive move semantics are not supported for this 
-                 * type. In such cases, we set the move_dtor as ctor_move_dtor, 
-                 * which indicates a destructive move operation. This adjustment 
-                 * ensures compatibility with different language bindings. */
-                if (!ti->hooks.move_ctor && ti->hooks.ctor_move_dtor) {
-                  move_dtor = ti->hooks.ctor_move_dtor;
+                 * type. In such cases when we are performing a non-destructive deletion, 
+                 * we set the move_dtor as ctor_move_dtor, which indicates we are operating
+                 * on memory already cleaned up. This adjustment ensures compatibility
+                 * with different language bindings. */
+                if (!destruct && !ti->hooks.move_ctor) {
+                    move_dtor = ti->hooks.ctor_move_dtor;
                 }
 
                 if (move_dtor) {
@@ -1986,8 +1987,14 @@ void flecs_table_move(
                 flecs_table_invoke_add_hooks(world, dst_table,
                     i_new, &dst_entity, dst_index, 1, construct);
             } else {
+                ecs_type_info_t *ti = dst_column->ti;
+
+                /* If the component doesn't have a move/move_ctor,
+                 * then it doesn't support non-destructive moves and must
+                 * be destructed during its removal.
+                 */
                 flecs_table_invoke_remove_hooks(world, src_table,
-                    src_column, &src_entity, src_index, 1, use_move_dtor);
+                    src_column, &src_entity, src_index, 1, use_move_dtor || !ti->hooks.move_ctor);
             }
         }
 
@@ -2001,8 +2008,11 @@ void flecs_table_move(
     }
 
     for (; (i_old < src_column_count); i_old ++) {
+        ecs_column_t *src_column = &src_columns[i_new];
+        ecs_type_info_t *ti = src_column->ti;
+
         flecs_table_invoke_remove_hooks(world, src_table, &src_columns[i_old], 
-            &src_entity, src_index, 1, use_move_dtor);
+            &src_entity, src_index, 1, use_move_dtor || !ti->hooks.move_ctor);
     }
 
     flecs_table_check_sanity(dst_table);

test/core/project.json

@@ -1359,6 +1359,9 @@
                 "batched_ensure_new_component_w_lifecycle",
                 "on_nested_prefab_copy_test_invokes_copy_count",
                 "no_move_no_move_ctor_with_move_dtor_with_ctor_move_dtor",
+                "dtor_on_destructive_component_removal",
+                "dtor_on_destructive_remove",
+                "dtor_on_destructive_component_removal_complex",
                 "new_w_table_ctor",
                 "new_w_table_on_add_hook",
                 "count_in_on_add",

test/core/src/ComponentLifecycle.c

@@ -3407,6 +3407,176 @@ void ComponentLifecycle_no_move_no_move_ctor_with_move_dtor_with_ctor_move_dtor(
     ecs_fini(world);
 }
 
+// For types that don't support non-destructive moves, make sure they call dtor appropriately.
+void ComponentLifecycle_dtor_on_destructive_component_removal(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+
+    cl_ctx pos_ctx = { { 0 } };
+
+    ecs_set_hooks(world, Position, {
+        .ctor = comp_ctor,
+        .move = NULL,
+        .move_ctor = NULL,
+        .dtor = comp_dtor,
+        .ctor_move_dtor = comp_pos_ctor_move_dtor,
+        .move_dtor = comp_move_dtor,
+        .ctx = &pos_ctx
+    });
+
+    pos_ctx.ctor.invoked = 0;
+    pos_ctx.dtor.invoked = 0;
+    pos_ctx.move_dtor.invoked = 0;
+
+    ecs_defer_begin(world);
+    ecs_entity_t e = ecs_new(world);
+    ecs_add(world, e, Position); 
+
+    ecs_entity_t e2 = ecs_new(world);
+    ecs_add(world, e2, Position);
+
+    ecs_entity_t e3 = ecs_new(world);
+    ecs_add(world, e3, Position);
+    ecs_defer_end(world);
+
+    test_int(pos_ctx.ctor.invoked, 3);
+    test_int(pos_ctx.move_dtor.invoked, 0);
+    test_int(pos_ctx.dtor.invoked, 0);
+
+    ecs_defer_begin(world);
+    ecs_remove(world, e, Position);
+    ecs_remove(world, e2, Position);
+    ecs_remove(world, e3, Position);
+    ecs_defer_end(world);
+
+    // Total deletes should be equal to 3
+    test_int(pos_ctx.move_dtor.invoked + pos_ctx.dtor.invoked, 3);
+
+    ecs_fini(world);
+}
+
+void ComponentLifecycle_dtor_on_destructive_remove(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+
+    cl_ctx pos_ctx = { { 0 } };
+
+    ecs_set_hooks(world, Position, {
+        .ctor = comp_ctor,
+        .move = NULL,
+        .move_ctor = NULL,
+        .dtor = comp_dtor,
+        .ctor_move_dtor = comp_pos_ctor_move_dtor,
+        .move_dtor = comp_move_dtor,
+        .ctx = &pos_ctx
+    });
+
+    pos_ctx.ctor.invoked = 0;
+    pos_ctx.dtor.invoked = 0;
+    pos_ctx.move_dtor.invoked = 0;
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add(world, e, Position); 
+
+    ecs_entity_t e2 = ecs_new(world);
+    ecs_add(world, e2, Position);
+
+    ecs_entity_t e3 = ecs_new(world);
+    ecs_add(world, e3, Position);
+
+    test_int(pos_ctx.ctor.invoked, 3);
+    test_int(pos_ctx.move_dtor.invoked, 0);
+    test_int(pos_ctx.dtor.invoked, 0);
+
+    ecs_defer_begin(world);
+    ecs_delete(world, e);
+    ecs_delete(world, e2);
+    ecs_delete(world, e3);
+    ecs_defer_end(world);
+
+    // Total deletes should be equal to 3
+    test_int(pos_ctx.move_dtor.invoked + pos_ctx.dtor.invoked, 3);
+
+    ecs_fini(world);
+}
+
+void ComponentLifecycle_dtor_on_destructive_component_removal_complex(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_COMPONENT(world, Velocity);
+
+    cl_ctx pos_ctx = { { 0 } };
+
+    ecs_set_hooks(world, Position, {
+        .ctor = comp_ctor,
+        .move = NULL,
+        .move_ctor = NULL,
+        .dtor = comp_dtor,
+        .ctor_move_dtor = comp_pos_ctor_move_dtor,
+        .move_dtor = comp_move_dtor,
+        .ctx = &pos_ctx
+    });
+
+    cl_ctx vel_ctx = { { 0 } };
+
+    ecs_set_hooks(world, Velocity, {
+        .ctor = comp_ctor,
+        .move = NULL,
+        .move_ctor = NULL,
+        .dtor = comp_dtor,
+        .ctor_move_dtor = comp_pos_ctor_move_dtor,
+        .move_dtor = comp_move_dtor,
+        .ctx = &vel_ctx
+    });
+
+    pos_ctx.ctor.invoked = 0;
+    pos_ctx.dtor.invoked = 0;
+    pos_ctx.move_dtor.invoked = 0;
+
+    vel_ctx.ctor.invoked = 0;
+    vel_ctx.dtor.invoked = 0;
+    vel_ctx.move_dtor.invoked = 0;
+
+    ecs_entity_t e = ecs_new(world);
+    ecs_add(world, e, Position); 
+
+    ecs_entity_t e2 = ecs_new(world);
+    ecs_add(world, e2, Position);
+
+    ecs_entity_t e3 = ecs_new(world);
+    ecs_add(world, e3, Position);
+
+    test_int(pos_ctx.ctor.invoked, 3);
+    test_int(pos_ctx.move_dtor.invoked, 0);
+    test_int(pos_ctx.dtor.invoked, 0);
+
+    ecs_add(world, e, Velocity);
+    // e3 is after e in the Position, Velocity archetype table.
+    ecs_add(world, e3, Velocity);
+
+    test_int(vel_ctx.ctor.invoked, 2);
+    test_int(vel_ctx.move_dtor.invoked, 0);
+    test_int(vel_ctx.dtor.invoked, 0);
+
+    test_int(pos_ctx.move_dtor.invoked, 0);
+    test_int(pos_ctx.dtor.invoked, 0);
+
+    ecs_remove(world, e, Position);
+
+    // e3 Velocity moves into e Velocity, e moves back to the old table.
+    // This should have no dtors because Velocity hasn't actually been deleted anywhere.
+    test_int(vel_ctx.move_dtor.invoked, 0);
+    test_int(vel_ctx.dtor.invoked, 0);
+
+    // Position should only have 1 dtor.
+    test_int(pos_ctx.move_dtor.invoked + pos_ctx.dtor.invoked, 1);
+
+    ecs_fini(world);
+}
+
 void ComponentLifecycle_new_w_table_ctor(void) {
     ecs_world_t *world = ecs_mini();
 
@@ -4353,3 +4523,11 @@ void ComponentLifecycle_shrink(void) {
 
     ecs_fini(world);
 }
+
+void ComponentLifecycle_move_dtor_on_set(void) {
+    // Implement testcase
+}
+
+void ComponentLifecycle_move_dtor_on_deferred_set(void) {
+    // Implement testcase
+}

test/core/src/main.c

@@ -1300,6 +1300,9 @@ void ComponentLifecycle_batched_set_new_component_w_lifecycle(void);
 void ComponentLifecycle_batched_ensure_new_component_w_lifecycle(void);
 void ComponentLifecycle_on_nested_prefab_copy_test_invokes_copy_count(void);
 void ComponentLifecycle_no_move_no_move_ctor_with_move_dtor_with_ctor_move_dtor(void);
+void ComponentLifecycle_dtor_on_destructive_component_removal(void);
+void ComponentLifecycle_dtor_on_destructive_remove(void);
+void ComponentLifecycle_dtor_on_destructive_component_removal_complex(void);
 void ComponentLifecycle_new_w_table_ctor(void);
 void ComponentLifecycle_new_w_table_on_add_hook(void);
 void ComponentLifecycle_count_in_on_add(void);
@@ -7828,6 +7831,18 @@ bake_test_case ComponentLifecycle_testcases[] = {
         "no_move_no_move_ctor_with_move_dtor_with_ctor_move_dtor",
         ComponentLifecycle_no_move_no_move_ctor_with_move_dtor_with_ctor_move_dtor
     },
+    {
+        "dtor_on_destructive_component_removal",
+        ComponentLifecycle_dtor_on_destructive_component_removal
+    },
+    {
+        "dtor_on_destructive_remove",
+        ComponentLifecycle_dtor_on_destructive_remove
+    },
+    {
+        "dtor_on_destructive_component_removal_complex",
+        ComponentLifecycle_dtor_on_destructive_component_removal_complex
+    },
     {
         "new_w_table_ctor",
         ComponentLifecycle_new_w_table_ctor
@@ -13956,7 +13971,7 @@ static bake_test_suite suites[] = {
         "ComponentLifecycle",
         ComponentLifecycle_setup,
         NULL,
-        132,
+        135,
         ComponentLifecycle_testcases
     },
     {