From 3dd7ce62f39da1e1249212b30d156407f3695186 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Wed, 8 Apr 2026 16:31:31 -0700 Subject: [PATCH] Set default move hook when only ctor/dtor are specified --- distr/flecs.c | 46 +++++--- distr/flecs.h | 18 +++- include/flecs/private/api_support.h | 18 +++- src/type_info.c | 46 +++++--- test/core/project.json | 8 +- test/core/src/ComponentLifecycle.c | 158 ++++++++++++++++++++++++++++ test/core/src/main.c | 32 +++++- test/meta/src/RuntimeTypes.c | 14 ++- 8 files changed, 299 insertions(+), 41 deletions(-) diff --git a/distr/flecs.c b/distr/flecs.c index 54e9532ee0..825ed53f04 100644 --- a/distr/flecs.c +++ b/distr/flecs.c @@ -20948,13 +20948,23 @@ void flecs_type_info_mark_in_use( #endif void flecs_default_ctor( - void *ptr, - int32_t count, + void *ptr, + int32_t count, const ecs_type_info_t *ti) { ecs_os_memset(ptr, 0, ti->size * count); } +void flecs_default_move( + void *dst_ptr, + void *src_ptr, + int32_t count, + const ecs_type_info_t *ti) +{ + ecs_os_memcpy(dst_ptr, src_ptr, ti->size * count); + ecs_os_memset(src_ptr, 0, ti->size * count); +} + bool flecs_type_info_ctor( void *ptr, int32_t count, @@ -21152,14 +21162,6 @@ void flecs_default_move_ctor_w_dtor(void *dst_ptr, void *src_ptr, cl->dtor(src_ptr, count, ti); } -static -void flecs_default_move(void *dst_ptr, void *src_ptr, - int32_t count, const ecs_type_info_t *ti) -{ - const ecs_type_hooks_t *cl = &ti->hooks; - cl->move(dst_ptr, src_ptr, count, ti); -} - static void flecs_default_dtor(void *dst_ptr, void *src_ptr, int32_t count, const ecs_type_info_t *ti) @@ -21424,7 +21426,21 @@ void ecs_set_hooks_id( * ease of use, if no constructor is specified, set a default one that * initializes the component to 0. */ if (!h->ctor && (h->dtor || h->copy || h->move)) { - ti->hooks.ctor = flecs_default_ctor; + ti->hooks.ctor = flecs_default_ctor; + } + + /* If only ctor and dtor are set, default move to flecs_default_move which + * memcpys src to dst and zeros src. This avoids invoking the dtor on the + * source after a move. Skip when ctor/dtor/move are flagged illegal: in + * those cases h->ctor / h->dtor may point to illegal stubs and pairing + * them with a move is not meaningful. */ + if (h->ctor && h->dtor && !h->move && !h->copy && !h->copy_ctor && + !h->move_ctor && !h->move_dtor && !h->ctor_move_dtor && + !(flags & (ECS_TYPE_HOOK_MOVE_ILLEGAL | + ECS_TYPE_HOOK_CTOR_ILLEGAL | + ECS_TYPE_HOOK_DTOR_ILLEGAL))) + { + ti->hooks.move = flecs_default_move; } /* Set default copy ctor, move ctor and merge */ @@ -21435,14 +21451,14 @@ void ecs_set_hooks_id( } if (!h->move_ctor && !(flags & ECS_TYPE_HOOK_MOVE_CTOR_ILLEGAL)) { - if (h->move) { + if (ti->hooks.move) { ti->hooks.move_ctor = flecs_default_move_ctor; } } if (!h->ctor_move_dtor) { ecs_flags32_t illegal_check = 0; - if (h->move) { + if (ti->hooks.move) { illegal_check |= ECS_TYPE_HOOK_MOVE_ILLEGAL; if (h->dtor) { illegal_check |= ECS_TYPE_HOOK_DTOR_ILLEGAL; @@ -21482,13 +21498,13 @@ void ecs_set_hooks_id( if (!h->move_dtor) { ecs_flags32_t illegal_check = 0; - if (h->move) { + if (ti->hooks.move) { illegal_check |= ECS_TYPE_HOOK_MOVE_ILLEGAL; if (h->dtor) { illegal_check |= ECS_TYPE_HOOK_DTOR_ILLEGAL; ti->hooks.move_dtor = flecs_default_move_w_dtor; } else { - ti->hooks.move_dtor = flecs_default_move; + ti->hooks.move_dtor = ti->hooks.move; } } else { if (h->dtor) { diff --git a/distr/flecs.h b/distr/flecs.h index cd7b4b6e2d..94e36bc650 100644 --- a/distr/flecs.h +++ b/distr/flecs.h @@ -4955,8 +4955,22 @@ char* flecs_module_path_from_c( */ FLECS_API void flecs_default_ctor( - void *ptr, - int32_t count, + void *ptr, + int32_t count, + const ecs_type_info_t *type_info); + +/** Move that memcpys src to dst and zero-initializes src. + * + * @param dst_ptr Pointer to the destination value. + * @param src_ptr Pointer to the source value. + * @param count Number of elements to move. + * @param type_info Type info for the component. + */ +FLECS_API +void flecs_default_move( + void *dst_ptr, + void *src_ptr, + int32_t count, const ecs_type_info_t *type_info); /* Wrapper functions for invoking type hooks with fallback behavior. */ diff --git a/include/flecs/private/api_support.h b/include/flecs/private/api_support.h index 10fc596cf6..e93dd2bb6e 100644 --- a/include/flecs/private/api_support.h +++ b/include/flecs/private/api_support.h @@ -48,8 +48,22 @@ char* flecs_module_path_from_c( */ FLECS_API void flecs_default_ctor( - void *ptr, - int32_t count, + void *ptr, + int32_t count, + const ecs_type_info_t *type_info); + +/** Move that memcpys src to dst and zero-initializes src. + * + * @param dst_ptr Pointer to the destination value. + * @param src_ptr Pointer to the source value. + * @param count Number of elements to move. + * @param type_info Type info for the component. + */ +FLECS_API +void flecs_default_move( + void *dst_ptr, + void *src_ptr, + int32_t count, const ecs_type_info_t *type_info); /* Wrapper functions for invoking type hooks with fallback behavior. */ diff --git a/src/type_info.c b/src/type_info.c index d39659b508..9e15df2384 100644 --- a/src/type_info.c +++ b/src/type_info.c @@ -18,13 +18,23 @@ void flecs_type_info_mark_in_use( #endif void flecs_default_ctor( - void *ptr, - int32_t count, + void *ptr, + int32_t count, const ecs_type_info_t *ti) { ecs_os_memset(ptr, 0, ti->size * count); } +void flecs_default_move( + void *dst_ptr, + void *src_ptr, + int32_t count, + const ecs_type_info_t *ti) +{ + ecs_os_memcpy(dst_ptr, src_ptr, ti->size * count); + ecs_os_memset(src_ptr, 0, ti->size * count); +} + bool flecs_type_info_ctor( void *ptr, int32_t count, @@ -222,14 +232,6 @@ void flecs_default_move_ctor_w_dtor(void *dst_ptr, void *src_ptr, cl->dtor(src_ptr, count, ti); } -static -void flecs_default_move(void *dst_ptr, void *src_ptr, - int32_t count, const ecs_type_info_t *ti) -{ - const ecs_type_hooks_t *cl = &ti->hooks; - cl->move(dst_ptr, src_ptr, count, ti); -} - static void flecs_default_dtor(void *dst_ptr, void *src_ptr, int32_t count, const ecs_type_info_t *ti) @@ -494,7 +496,21 @@ void ecs_set_hooks_id( * ease of use, if no constructor is specified, set a default one that * initializes the component to 0. */ if (!h->ctor && (h->dtor || h->copy || h->move)) { - ti->hooks.ctor = flecs_default_ctor; + ti->hooks.ctor = flecs_default_ctor; + } + + /* If only ctor and dtor are set, default move to flecs_default_move which + * memcpys src to dst and zeros src. This avoids invoking the dtor on the + * source after a move. Skip when ctor/dtor/move are flagged illegal: in + * those cases h->ctor / h->dtor may point to illegal stubs and pairing + * them with a move is not meaningful. */ + if (h->ctor && h->dtor && !h->move && !h->copy && !h->copy_ctor && + !h->move_ctor && !h->move_dtor && !h->ctor_move_dtor && + !(flags & (ECS_TYPE_HOOK_MOVE_ILLEGAL | + ECS_TYPE_HOOK_CTOR_ILLEGAL | + ECS_TYPE_HOOK_DTOR_ILLEGAL))) + { + ti->hooks.move = flecs_default_move; } /* Set default copy ctor, move ctor and merge */ @@ -505,14 +521,14 @@ void ecs_set_hooks_id( } if (!h->move_ctor && !(flags & ECS_TYPE_HOOK_MOVE_CTOR_ILLEGAL)) { - if (h->move) { + if (ti->hooks.move) { ti->hooks.move_ctor = flecs_default_move_ctor; } } if (!h->ctor_move_dtor) { ecs_flags32_t illegal_check = 0; - if (h->move) { + if (ti->hooks.move) { illegal_check |= ECS_TYPE_HOOK_MOVE_ILLEGAL; if (h->dtor) { illegal_check |= ECS_TYPE_HOOK_DTOR_ILLEGAL; @@ -552,13 +568,13 @@ void ecs_set_hooks_id( if (!h->move_dtor) { ecs_flags32_t illegal_check = 0; - if (h->move) { + if (ti->hooks.move) { illegal_check |= ECS_TYPE_HOOK_MOVE_ILLEGAL; if (h->dtor) { illegal_check |= ECS_TYPE_HOOK_DTOR_ILLEGAL; ti->hooks.move_dtor = flecs_default_move_w_dtor; } else { - ti->hooks.move_dtor = flecs_default_move; + ti->hooks.move_dtor = ti->hooks.move; } } else { if (h->dtor) { diff --git a/test/core/project.json b/test/core/project.json index 30227643c5..a7dce60eb5 100644 --- a/test/core/project.json +++ b/test/core/project.json @@ -1714,7 +1714,13 @@ "has_in_on_add_hook_move", "get_in_on_add_hook_new", "get_in_on_add_hook_move", - "get_name_in_on_add_hook_move" + "get_name_in_on_add_hook_move", + "default_move_copies_and_zeros_src", + "default_move_copies_and_zeros_src_count", + "set_hooks_ctor_dtor_assigns_default_move", + "set_hooks_ctor_only_no_default_move", + "set_hooks_ctor_dtor_with_move_keeps_user_move", + "set_hooks_ctor_dtor_cascades_move_hooks" ] }, { "id": "Pairs", diff --git a/test/core/src/ComponentLifecycle.c b/test/core/src/ComponentLifecycle.c index 1c15520f42..36807be877 100644 --- a/test/core/src/ComponentLifecycle.c +++ b/test/core/src/ComponentLifecycle.c @@ -4546,3 +4546,161 @@ void ComponentLifecycle_get_name_in_on_add_hook_move(void) { ecs_fini(world); } + +void ComponentLifecycle_default_move_copies_and_zeros_src(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + const ecs_type_info_t *ti = ecs_get_type_info(world, ecs_id(Position)); + test_assert(ti != NULL); + + Position src = {10, 20}; + Position dst = {0, 0}; + + flecs_default_move(&dst, &src, 1, ti); + + test_int(dst.x, 10); + test_int(dst.y, 20); + test_int(src.x, 0); + test_int(src.y, 0); + + ecs_fini(world); +} + +void ComponentLifecycle_default_move_copies_and_zeros_src_count(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + const ecs_type_info_t *ti = ecs_get_type_info(world, ecs_id(Position)); + test_assert(ti != NULL); + + Position src[3] = {{1, 2}, {3, 4}, {5, 6}}; + Position dst[3] = {{0}}; + + flecs_default_move(dst, src, 3, ti); + + test_int(dst[0].x, 1); test_int(dst[0].y, 2); + test_int(dst[1].x, 3); test_int(dst[1].y, 4); + test_int(dst[2].x, 5); test_int(dst[2].y, 6); + + test_int(src[0].x, 0); test_int(src[0].y, 0); + test_int(src[1].x, 0); test_int(src[1].y, 0); + test_int(src[2].x, 0); test_int(src[2].y, 0); + + ecs_fini(world); +} + +static int default_move_dtor_invoked = 0; +static +void default_move_test_dtor( + void *ptr, + int32_t count, + const ecs_type_info_t *info) +{ + (void)ptr; + (void)info; + default_move_dtor_invoked += count; +} + +void ComponentLifecycle_set_hooks_ctor_dtor_assigns_default_move(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_set_hooks(world, Position, { + .ctor = flecs_default_ctor, + .dtor = default_move_test_dtor + }); + + const ecs_type_hooks_t *hooks = ecs_get_hooks_id(world, ecs_id(Position)); + test_assert(hooks != NULL); + test_assert(hooks->ctor == flecs_default_ctor); + test_assert(hooks->dtor == default_move_test_dtor); + test_assert(hooks->move == flecs_default_move); + test_assert(!(hooks->flags & ECS_TYPE_HOOK_MOVE_ILLEGAL)); + + ecs_fini(world); +} + +void ComponentLifecycle_set_hooks_ctor_only_no_default_move(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_set_hooks(world, Position, { + .ctor = flecs_default_ctor + }); + + const ecs_type_hooks_t *hooks = ecs_get_hooks_id(world, ecs_id(Position)); + test_assert(hooks != NULL); + test_assert(hooks->ctor == flecs_default_ctor); + test_assert(hooks->move == NULL); + + ecs_fini(world); +} + +static +void default_move_test_move( + void *dst_ptr, + void *src_ptr, + int32_t count, + const ecs_type_info_t *info) +{ + memcpy(dst_ptr, src_ptr, info->size * count); +} + +void ComponentLifecycle_set_hooks_ctor_dtor_with_move_keeps_user_move(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + + ecs_set_hooks(world, Position, { + .ctor = flecs_default_ctor, + .dtor = default_move_test_dtor, + .move = default_move_test_move + }); + + const ecs_type_hooks_t *hooks = ecs_get_hooks_id(world, ecs_id(Position)); + test_assert(hooks != NULL); + test_assert(hooks->move == default_move_test_move); + + ecs_fini(world); +} + +void ComponentLifecycle_set_hooks_ctor_dtor_cascades_move_hooks(void) { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT(world, Position); + ECS_TAG(world, Tag); + + ecs_set_hooks(world, Position, { + .ctor = flecs_default_ctor, + .dtor = default_move_test_dtor + }); + + const ecs_type_hooks_t *hooks = ecs_get_hooks_id(world, ecs_id(Position)); + test_assert(hooks != NULL); + test_assert(hooks->move == flecs_default_move); + test_assert(hooks->move_ctor != NULL); + test_assert(hooks->ctor_move_dtor != NULL); + test_assert(hooks->move_dtor != NULL); + test_assert(!(hooks->flags & ECS_TYPE_HOOK_MOVE_ILLEGAL)); + test_assert(!(hooks->flags & ECS_TYPE_HOOK_MOVE_CTOR_ILLEGAL)); + test_assert(!(hooks->flags & ECS_TYPE_HOOK_CTOR_MOVE_DTOR_ILLEGAL)); + test_assert(!(hooks->flags & ECS_TYPE_HOOK_MOVE_DTOR_ILLEGAL)); + + ecs_entity_t e = ecs_new_w(world, Position); + ecs_set(world, e, Position, {10, 20}); + ecs_add(world, e, Tag); + + { + const Position *p = ecs_get(world, e, Position); + test_assert(p != NULL); + test_int(p->x, 10); + test_int(p->y, 20); + } + + ecs_fini(world); +} diff --git a/test/core/src/main.c b/test/core/src/main.c index c29b384a17..099b9268b2 100644 --- a/test/core/src/main.c +++ b/test/core/src/main.c @@ -1654,6 +1654,12 @@ void ComponentLifecycle_has_in_on_add_hook_move(void); void ComponentLifecycle_get_in_on_add_hook_new(void); void ComponentLifecycle_get_in_on_add_hook_move(void); void ComponentLifecycle_get_name_in_on_add_hook_move(void); +void ComponentLifecycle_default_move_copies_and_zeros_src(void); +void ComponentLifecycle_default_move_copies_and_zeros_src_count(void); +void ComponentLifecycle_set_hooks_ctor_dtor_assigns_default_move(void); +void ComponentLifecycle_set_hooks_ctor_only_no_default_move(void); +void ComponentLifecycle_set_hooks_ctor_dtor_with_move_keeps_user_move(void); +void ComponentLifecycle_set_hooks_ctor_dtor_cascades_move_hooks(void); // Testsuite 'Pairs' void Pairs_type_w_one_pair(void); @@ -9681,6 +9687,30 @@ bake_test_case ComponentLifecycle_testcases[] = { { "get_name_in_on_add_hook_move", ComponentLifecycle_get_name_in_on_add_hook_move + }, + { + "default_move_copies_and_zeros_src", + ComponentLifecycle_default_move_copies_and_zeros_src + }, + { + "default_move_copies_and_zeros_src_count", + ComponentLifecycle_default_move_copies_and_zeros_src_count + }, + { + "set_hooks_ctor_dtor_assigns_default_move", + ComponentLifecycle_set_hooks_ctor_dtor_assigns_default_move + }, + { + "set_hooks_ctor_only_no_default_move", + ComponentLifecycle_set_hooks_ctor_only_no_default_move + }, + { + "set_hooks_ctor_dtor_with_move_keeps_user_move", + ComponentLifecycle_set_hooks_ctor_dtor_with_move_keeps_user_move + }, + { + "set_hooks_ctor_dtor_cascades_move_hooks", + ComponentLifecycle_set_hooks_ctor_dtor_cascades_move_hooks } }; @@ -16173,7 +16203,7 @@ static bake_test_suite suites[] = { "ComponentLifecycle", ComponentLifecycle_setup, NULL, - 139, + 145, ComponentLifecycle_testcases }, { diff --git a/test/meta/src/RuntimeTypes.c b/test/meta/src/RuntimeTypes.c index 9d05155838..7101cffd00 100644 --- a/test/meta/src/RuntimeTypes.c +++ b/test/meta/src/RuntimeTypes.c @@ -330,8 +330,11 @@ void RuntimeTypes_dtor(void) { test_assert(test_struct_ti->hooks.ctor != NULL); test_assert(test_struct_ti->hooks.dtor != NULL); - /* No other hooks should've been set: */ - test_assert(test_struct_ti->hooks.move == NULL); + /* When a member has a (ctor + dtor), set_hooks auto-assigns a default + * move on the member type. The struct generator then propagates a move + * hook to TestStruct so archetype moves of members with dtors are safe + * (move zeros src so subsequent dtor on src is harmless). */ + test_assert(test_struct_ti->hooks.move != NULL); test_assert(test_struct_ti->hooks.copy == NULL); /* Now instantiate TestStruct to test its destructor. */ @@ -957,9 +960,10 @@ void RuntimeTypes_array_dtor(void) { test_assert(hooks->dtor != NULL); /* should be set to call the resource handle dtor for each item in the array. - no other hooks should have been generated, since the depending type, - "resource_handle" only has a dtor hook: */ - test_assert(hooks->move == NULL); + set_hooks auto-assigns a default move (memcpy + zero src) on the + element type when only ctor + dtor are set, so the array generator + propagates a move hook here too: */ + test_assert(hooks->move != NULL); test_assert(hooks->copy == NULL); /* Test that the set dtor hook is indeed working. */