From 75180f718a84007f95224d4d1b508c2a05acdedb Mon Sep 17 00:00:00 2001 From: "Jonathan B. Coe" Date: Sat, 25 Jul 2026 18:17:05 +0000 Subject: [PATCH] Add narrowing conversions: A to A_Subset, concurrency stress test protocol_owning_vtable_traits/map_owning_vtable_members become generic instead of per-interface generated code, enabling narrowing conversions like A to A_Subset. Fixes vtable_slot_name to qualify by member signature only, not declaring class, since narrowing needs the same conceptual member to resolve to the same name across interfaces. --- CMakeLists.txt | 3 + protocol_reflection.hxx | 114 +++++++++++++++++ protocol_reflection_detail/naming.hxx | 34 ++++-- protocol_reflection_smoke_test.cc | 170 ++++++++++++++++++++++++++ 4 files changed, 313 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bea543..94ab492 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -248,10 +248,13 @@ if(XYZ_PROTOCOL_IS_NOT_SUBPROJECT) protocol_reflection_smoke_test VERSION 26 + LINK_LIBRARIES + protocol FILES protocol.h protocol_reflection.hxx interface_A.h + interface_A_Subset.h interface_B.h interface_C.h tracking_allocator.h diff --git a/protocol_reflection.hxx b/protocol_reflection.hxx index bf4ed14..06e9433 100644 --- a/protocol_reflection.hxx +++ b/protocol_reflection.hxx @@ -143,10 +143,48 @@ using protocol_bases = typename[:protocol_bases_type():]; } // namespace reflection_detail +// protocol's own nested vtable already is the owning +// vtable used for narrowing; this trait just names it the way +// protocol.h's get_owning_vtable expects, matching how the Python/libclang +// backend's own generated protocol_owning_vtable_traits<::xyz::A, +// Allocator> is a bare alias to protocol<::xyz::A, Allocator>::vtable, not +// a separately-designed type. +template +struct protocol_owning_vtable_traits { + using vtable = typename protocol::vtable; +}; + +// Narrows a source owning vtable to a target one by copying every entry +// the target has (by exact name -- including xyz_protocol_clone/_move/ +// _destroy, which every protocol's own vtable always declares) from the +// matching entry in the source. FromVtable/ToVtable are deduced directly +// from the pointer arguments, not from an Interface/Allocator template +// argument list: unlike the Python/libclang backend, where a per-interface +// generated overload hardcodes its own target type, this one generic +// definition works for any (FromInterface, ToInterface, Allocator), since +// it reflects on the two concrete vtable struct types themselves rather +// than needing to know which interfaces they came from. Found via ADL +// from get_owning_vtable (protocol.h): FromVtable/ToVtable are nested +// types of xyz::protocol<...>, so this must live in namespace xyz, not +// xyz::reflection_detail, to be visible there. +template +void map_owning_vtable_members(const FromVtable* from, ToVtable* to) { + template for (constexpr std::meta::info to_member : + std::define_static_array(std::meta::nonstatic_data_members_of( + ^^ToVtable, std::meta::access_context::current()))) { + constexpr std::string_view name = std::meta::identifier_of(to_member); + constexpr std::meta::info from_member = + reflection_detail::find_data_member(^^FromVtable, name); + to->[:to_member:] = from->[:from_member:]; + } +} + template class protocol : public reflection_detail::protocol_bases { template friend struct reflection_detail::protocol_single_overload_wrapper; + template + friend class protocol; using clone_or_move_fn = void* (*)(void*, const Allocator&); using destroy_fn = void (*)(void*, const Allocator&); @@ -339,6 +377,82 @@ class protocol : public reflection_detail::protocol_bases { allocator_traits::is_always_equal::value) : protocol(std::allocator_arg_t{}, other.alloc_, std::move(other)) {} + template + requires(!std::same_as) + constexpr protocol(std::allocator_arg_t, const Allocator& alloc, + const protocol& other) + : alloc_(alloc) { + if (!other.valueless_after_move()) { + p_ = other.vtable_->xyz_protocol_clone(other.p_, alloc_); + vtable_ = get_owning_vtable(other.vtable_); + } else { + p_ = nullptr; + vtable_ = nullptr; + } + } + + template + requires(!std::same_as) + constexpr protocol(const protocol& other) + : alloc_(allocator_traits::select_on_container_copy_construction( + other.alloc_)) { + if (!other.valueless_after_move()) { + p_ = other.vtable_->xyz_protocol_clone(other.p_, alloc_); + vtable_ = get_owning_vtable(other.vtable_); + } else { + p_ = nullptr; + vtable_ = nullptr; + } + } + + template + requires(!std::same_as) + constexpr protocol( + std::allocator_arg_t, const Allocator& alloc, + protocol&& + other) noexcept(allocator_traits::is_always_equal::value) + : alloc_(alloc) { + if (alloc_ == other.alloc_) { + p_ = std::exchange(other.p_, nullptr); + vtable_ = get_owning_vtable( + std::exchange(other.vtable_, nullptr)); + } else { + if (!other.valueless_after_move()) { + p_ = other.vtable_->xyz_protocol_move(other.p_, alloc_); + vtable_ = get_owning_vtable(other.vtable_); + other.vtable_->xyz_protocol_destroy(other.p_, other.alloc_); + other.p_ = nullptr; + other.vtable_ = nullptr; + } else { + p_ = nullptr; + vtable_ = nullptr; + } + } + } + + template + requires(!std::same_as) + constexpr protocol(protocol&& other) noexcept( + allocator_traits::is_always_equal::value) + : alloc_(other.alloc_) { + if (alloc_ == other.alloc_) { + p_ = std::exchange(other.p_, nullptr); + vtable_ = get_owning_vtable( + std::exchange(other.vtable_, nullptr)); + } else { + if (!other.valueless_after_move()) { + p_ = other.vtable_->xyz_protocol_move(other.p_, alloc_); + vtable_ = get_owning_vtable(other.vtable_); + other.vtable_->xyz_protocol_destroy(other.p_, other.alloc_); + other.p_ = nullptr; + other.vtable_ = nullptr; + } else { + p_ = nullptr; + vtable_ = nullptr; + } + } + } + constexpr bool valueless_after_move() const noexcept { return p_ == nullptr; } ~protocol() { diff --git a/protocol_reflection_detail/naming.hxx b/protocol_reflection_detail/naming.hxx index ba36309..0abc45b 100644 --- a/protocol_reflection_detail/naming.hxx +++ b/protocol_reflection_detail/naming.hxx @@ -29,6 +29,8 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include #include +#include "protocol_reflection_detail/types.hxx" + namespace xyz::reflection_detail { // `identifier_safe_string` must run at compile time. @@ -68,15 +70,31 @@ consteval std::string vtable_entry_name(std::meta::info member) { return std::string(std::meta::identifier_of(member)); } -// A generated vtable struct's entry name for `member`, unique per exact -// overload. Unlike vtable_entry_name above, this qualifies by the member's -// full display string (return type, parameter types, constness), not just -// its identifier: a vtable struct is an internal, never user-visible -// implementation detail with one data member per overload, so two -// overloads sharing a name (e.g. interface C's three `compute`s) need two -// distinct entries here, even though they share one forwarder name. +// A vtable struct's entry name for `member`, unique per exact overload +// (unlike vtable_entry_name above): interface C's overloaded `compute`s +// need one distinct entry each despite sharing one forwarder name. +// +// Built from the return type, parameter types, and constness individually, +// not from std::meta::display_string_of(member): a member's own display +// string is qualified by its declaring class (e.g. "int A::compute(int)"), +// which would make A::name and A_Subset::name get different names despite +// being the same conceptual member for narrowing's cross-interface matching +// to find. consteval std::string vtable_slot_name(std::meta::info member) { - return identifier_safe_string(std::meta::display_string_of(member)); + std::string result = + identifier_safe_string(std::string(std::meta::identifier_of(member))); + result += "_"; + result += identifier_safe_string(std::meta::display_string_of( + std::meta::dealias(std::meta::return_type_of(member)))); + for (std::meta::info parameter_type : parameter_types_of(member)) { + result += "_"; + result += + identifier_safe_string(std::meta::display_string_of(parameter_type)); + } + if (std::meta::is_const(member)) { + result += "_const"; + } + return result; } // The data member of `struct_type` named `name`, for reading or writing diff --git a/protocol_reflection_smoke_test.cc b/protocol_reflection_smoke_test.cc index ce80539..0d85027 100644 --- a/protocol_reflection_smoke_test.cc +++ b/protocol_reflection_smoke_test.cc @@ -21,14 +21,17 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. // interface, keeping this file's compile/link surface minimal. #include +#include #include #include #include #include +#include #include #include #include "interface_A.h" +#include "interface_A_Subset.h" #include "interface_B.h" #include "interface_C.h" #include "protocol.h" @@ -297,4 +300,171 @@ TEST(ProtocolReflectionSmoke, AllocatorExtendedMoveFromValueless) { EXPECT_TRUE(ppp.valueless_after_move()); } +// Narrowing conversions: A (name, count) -> A_Subset (name only), the +// machinery wired into protocol_reflection.hxx's converting constructors +// and get_owning_vtable/get_mapped_vtable (protocol.cc, backend-agnostic). + +TEST(ProtocolReflectionSmoke, NarrowingCopyConversion) { + xyz::protocol> p( + std::allocator_arg, std::allocator(), + std::in_place_type); + xyz::protocol> p_subset(p); + EXPECT_FALSE(p.valueless_after_move()); + EXPECT_FALSE(p_subset.valueless_after_move()); + EXPECT_EQ(p.name(), "ALike"); + EXPECT_EQ(p_subset.name(), "ALike"); +} + +TEST(ProtocolReflectionSmoke, NarrowingMoveConversionEqualAllocators) { + xyz::protocol> p( + std::allocator_arg, std::allocator(), + std::in_place_type); + xyz::protocol> p_subset = + std::move(p); + EXPECT_TRUE(p.valueless_after_move()); + EXPECT_FALSE(p_subset.valueless_after_move()); + EXPECT_EQ(p_subset.name(), "ALike"); +} + +TEST(ProtocolReflectionSmoke, NarrowingCopyConversionFromValueless) { + xyz::protocol> p( + std::allocator_arg, std::allocator(), + std::in_place_type); + xyz::protocol> p2 = std::move(p); + EXPECT_TRUE(p.valueless_after_move()); + xyz::protocol> p_subset(p); + EXPECT_TRUE(p_subset.valueless_after_move()); +} + +TEST(ProtocolReflectionSmoke, CountAllocationsForNarrowingCopyConversion) { + unsigned alloc_counter = 0; + unsigned dealloc_counter = 0; + { + xyz::protocol> p( + std::allocator_arg, + xyz::TrackingAllocator(&alloc_counter, &dealloc_counter), + std::in_place_type); + EXPECT_EQ(alloc_counter, 1u); + xyz::protocol> p_subset(p); + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 0u); + } + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 2u); +} + +TEST(ProtocolReflectionSmoke, CountAllocationsForNarrowingMoveConversion) { + unsigned alloc_counter = 0; + unsigned dealloc_counter = 0; + { + xyz::protocol> a( + std::allocator_arg, + xyz::TrackingAllocator(&alloc_counter, &dealloc_counter), + std::in_place_type); + // Equal allocators: the pointer is stolen, no new allocation. + xyz::protocol> aa( + std::move(a)); + EXPECT_TRUE(a.valueless_after_move()); + EXPECT_FALSE(aa.valueless_after_move()); + EXPECT_EQ(alloc_counter, 1u); + EXPECT_EQ(dealloc_counter, 0u); + } + EXPECT_EQ(alloc_counter, 1u); + EXPECT_EQ(dealloc_counter, 1u); +} + +TEST(ProtocolReflectionSmoke, NarrowingCopyConversionNonEqualAllocators) { + unsigned alloc_counter = 0; + unsigned dealloc_counter = 0; + { + xyz::protocol> p( + std::allocator_arg, + NonEqualTrackingAllocator(&alloc_counter, &dealloc_counter), + std::in_place_type); + EXPECT_EQ(alloc_counter, 1u); + NonEqualTrackingAllocator target_alloc(&alloc_counter, + &dealloc_counter); + xyz::protocol> p_subset( + std::allocator_arg, target_alloc, p); + EXPECT_FALSE(p.valueless_after_move()); + EXPECT_FALSE(p_subset.valueless_after_move()); + EXPECT_EQ(p_subset.name(), "ALike"); + // 1 allocation for the source, 1 more for the cloned target. + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 0u); + } + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 2u); +} + +TEST(ProtocolReflectionSmoke, NarrowingMoveConversionNonEqualAllocators) { + unsigned alloc_counter = 0; + unsigned dealloc_counter = 0; + { + xyz::protocol> p( + std::allocator_arg, + NonEqualTrackingAllocator(&alloc_counter, &dealloc_counter), + std::in_place_type); + EXPECT_EQ(alloc_counter, 1u); + // Non-equal allocators force move-construction into new storage on the + // target's allocator, and the source is destroyed immediately. + xyz::protocol> p_subset( + std::move(p)); + EXPECT_TRUE(p.valueless_after_move()); + EXPECT_FALSE(p_subset.valueless_after_move()); + EXPECT_EQ(p_subset.name(), "ALike"); + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 1u); + } + EXPECT_EQ(alloc_counter, 2u); + EXPECT_EQ(dealloc_counter, 2u); +} + +TEST(ProtocolReflectionSmoke, NarrowingMoveConversionFromValueless) { + xyz::protocol> p( + std::allocator_arg, std::allocator(), + std::in_place_type); + xyz::protocol> p2 = std::move(p); + EXPECT_TRUE(p.valueless_after_move()); + xyz::protocol> p_subset = + std::move(p); + EXPECT_TRUE(p_subset.valueless_after_move()); +} + +// Owning-conversion slice of protocol_test.cc's +// NarrowingConversionConcurrentStressing: exercises protocol.cc's +// get_mapped_vtable caching/locking under contention. +TEST(ProtocolReflectionSmoke, NarrowingConversionConcurrentStressing) { + constexpr int kNumThreads = 20; + constexpr int kIterationsPerThread = 50; + std::vector threads; + std::atomic start_signal{false}; + + for (int i = 0; i < kNumThreads; ++i) { + threads.emplace_back([&start_signal]() { + while (!start_signal.load()) { + std::this_thread::yield(); + } + for (int iter = 0; iter < kIterationsPerThread; ++iter) { + xyz::protocol> p( + std::in_place_type); + xyz::protocol> p_subset = + std::move(p); + EXPECT_EQ(p_subset.name(), "ALike"); + + xyz::protocol> p_char( + std::in_place_type); + xyz::protocol> p_subset_char = + std::move(p_char); + EXPECT_EQ(p_subset_char.name(), "ALike"); + } + }); + } + + start_signal.store(true); + for (auto& thread : threads) { + thread.join(); + } +} + } // namespace