Simple tests for supported special member functions

[jbcoe]

First step in adding a reflection-based implementation of protocol (and protocol_view).

No reflection features are currently needed, just C++ 20 concepts and requirement checks to conditionally generate constructors.

Once this is landed we can extend the tests to document and test-drive a full implementation.

GTEST_SKIP() << "Skipping test"; can be used to skip tests while we are waiting to add implementation support.

Once a complete implementation has been submitted, we can retire the Python implementation.

[Copilot]

Pull request overview

Introduces an initial “reflection” scaffold for protocol / protocol_view, focused on compile-time constraints for special member functions and accompanying tests as a foundation for a future reflection-based implementation.

Changes:

• Adds xyz::reflection::protocol and xyz::reflection::protocol_view declarations with constrained special member functions.
• Adds GoogleTest-based compile-time trait checks documenting expected special-member availability.
• Wires the new reflection header + tests into the CMake build when testing is enabled.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
reflection/protocol.h	Adds initial reflection-based protocol/protocol_view declarations (currently header-only).
reflection/protocol_test.cc	Adds compile-time tests asserting special member function support expectations.
reflection/CMakeLists.txt	Defines an INTERFACE target for the reflection header and a test target.
CMakeLists.txt	Adds the reflection subdirectory to the test-enabled build configuration.

Suppressed comments (4)

reflection/protocol_test.cc:26

• Test suite name abbreviates “Reflection” as “R”, but the project guidelines require avoiding abbreviations in names. Consider using a fully spelled-out suite name.

TEST(RProtocolViewTest, CheckSpecialMembersForStructWithDeletedSpecialMembers) {

reflection/protocol_test.cc:46

• Test suite name abbreviates “Reflection” as “R”, but the project guidelines require avoiding abbreviations in names. Consider using a fully spelled-out suite name.

TEST(RProtocolTest, CheckSpecialMembers) {

reflection/protocol_test.cc:58

• Test suite name abbreviates “Reflection” as “R”, but the project guidelines require avoiding abbreviations in names. Consider using a fully spelled-out suite name.

TEST(RProtocolTest, CheckSpecialMembersForStructWithDeletedSpecialMembers) {

reflection/protocol_test.cc:60

• In these comments, “deleted” is misleading (objects are destroyed/destructed, not “deleted”). Consider rewording to avoid confusion about dynamic allocation.

  // protocol is not default-constructible and cannot be copied, moved,
  // assigned, move assigned and deleted if the underlying type cannot be.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jbcoe]

@copilot Fix the failing tests by restoring the missing definition of struct A {};

[Copilot]

@copilot Fix the failing tests by restoring the missing definition of struct A {};

Addressed in 468b4c6. struct A {} is restored at local scope in the tests that use it, and the branch now builds and tests cleanly.

[RyanJK5]

Looks good overall 👍

[RyanJK5]

Does an interface with a defaulted copy constructor imply the protocol object should be default constructible? e.g.

struct I {
    I() = default;
};

protocol<I> p{}; // Now has a valid empty state

I thought I noticed something like that in the original implementation, but I may very well be mistaken.

[jbcoe]

This needs a rethink but for now I think should be suppressed.

[jbcoe]

My thinking here is that for polymorphic, interfaces are often pure virtual whereas the design of protocol is to use structs without implementations of functions. Requiring the user to explicitly delete default construction seems unhelpful and later constructors (in place type) will allow construction from any conformant type.

[RyanJK5]

Makes sense, and also possible to change later if need be. It would also complicate matters, since in the default constructor case, valueless_after_move would be a misnomer (it could have been empty because it was constructed that way). duck similarly avoids the complexity by disallowing default construction.

[jbcoe]

@copilot look at https://eel.is/c++draft/mem.composite.types and update protocol constructor requirements to model the class template polymorphic

[RyanJK5]

I also pushed a change to add noexcept to protocol_view's move constructor and assignment, I can roll it back if that's unwanted. I withheld adding it to protocol since according to my understanding, that will depend on how the allocator type specifies POCMA and is_always_equal.

[jbcoe]

@RyanJK5 Looks good. As a first step this is easy to approve. We can fine-tune in later PRs.

[Twon]

Adding ${CMAKE_CURRENT_SOURCE_DIR} to files in the current directory is only necessary for CMake version proior to 3.13, for details see: https://crascit.com/2016/01/31/enhanced-source-file-handling-with-target_sources/

Suggested change

	$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/protocol.h>)
	$<BUILD_INTERFACE:protocol.h>)

[jbcoe]

The amended code gives me

CMake Error in reflection/CMakeLists.txt:
  Target "reflection_protocol" contains relative path in its
  INTERFACE_SOURCES:

    "protocol.h"

We've used the same pattern in CMakeLists.txt at the project root. Perhaps they can all be cleaned up in sync later?

[Twon]

A few minor comments but otherwise looks good