You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reflection core derives member offsets in two fragile ways:
OffsetFromStart (include/reflection.h:97) memcpys the raw bytes of a pointer-to-member and reinterprets the leading sizeof(size_t) bytes as a byte offset.
DEFINE_MEMBER (include/reflection.h:114) uses offsetof(struct REFLECTABLE, R), and members are accessed via reinterpret_cast at the computed offset (GetMemberAddress).
Impact
This works for simple standard-layout structs under the common Itanium / MSVC ABIs, but:
The byte representation of a pointer-to-member is implementation-defined; for multiple or virtual inheritance the leading-bytes-as-offset assumption is wrong.
offsetof on a non-standard-layout type is conditionally-supported / undefined.
So the whole introspection mechanism rests on assumptions the standard doesn't guarantee, and would break (silently, with wrong offsets) for records that aren't simple standard-layout aggregates.
Suggested direction
Document the constraint explicitly (reflectable records must be standard-layout, no inheritance) and enforce it at registration with a static_assert, so a violating type fails to compile instead of misbehaving at runtime.
Implementation note — the exact static_assert needs care (portability + C++11)
Two corrections to the naive static_assert(std::is_standard_layout_v<REFLECTABLE>):
C++11: the project's minimum is C++11, so use std::is_standard_layout<REFLECTABLE>::value
(the _v alias is C++17). <type_traits> must be included.
is_standard_layout on the record is implementation-dependent and may break CI. A class is
standard-layout only if all its non-static data members are themselves standard-layout — so is_standard_layout<REFLECTABLE> is only true when std::wstring (every MEMBER_TEXT) and TimePoint (MEMBER_DATETIME) are standard-layout, which varies by standard-library
implementation. Measured here: on libstdc++ (GCC/Clang on Linux) is_standard_layout<std::wstring>
and a record with wstring members are both true. libc++ (macOS) and MSVC (Windows) were NOT
verifiable in this environment. If either makes std::wstring/TimePoint non-standard-layout,
asserting is_standard_layout<REFLECTABLE> would fail to compile on that platform even for
perfectly valid records — a CI regression, not a safety win. (Note offsetof is already used on
these same types and works on all three CI compilers today, so the "UB" is theoretical for the
supported toolchains.)
Recommended enforcement
Portable, always-safe (do this):static_assert(!std::is_polymorphic<REFLECTABLE>::value, "reflectable records must not be polymorphic (no virtual functions/inheritance)"); — this does not
depend on stdlib string layout (a struct with wstring members is not polymorphic), and it catches
the realistic footgun that actually breaks the pointer-to-member/offset hacks (a record made
virtual or given a vtable). Place it where the complete REFLECTABLE type is visible (e.g. inside
the generated Register<REFLECTABLE>() in include/reflection.h).
Stronger is_standard_layout assert — only if verified: add static_assert(std::is_standard_layout<REFLECTABLE>::value, ...) ONLY after confirming it holds for
the test records on ALL THREE CI toolchains (Ubuntu/GCC, macOS/Clang, Windows/MSVC, C++11 and C++20).
If any platform fails it, do not add it — keep the polymorphic guard and document the constraint
instead, and leave this part as blocked (arguably needs the text representation to move off std::wstring, cf. std::wstring + deprecated <codecvt> conversion is non-portable (BMP-only on Windows) #25).
Document the constraint (reflectable records must be simple structs: no inheritance, no virtuals,
standard-layout members) in the README and/or the reflection macro header, so the requirement is
explicit regardless of which asserts are enforceable.
Problem
The reflection core derives member offsets in two fragile ways:
OffsetFromStart(include/reflection.h:97)memcpys the raw bytes of a pointer-to-member and reinterprets the leadingsizeof(size_t)bytes as a byte offset.DEFINE_MEMBER(include/reflection.h:114) usesoffsetof(struct REFLECTABLE, R), and members are accessed viareinterpret_castat the computed offset (GetMemberAddress).Impact
This works for simple standard-layout structs under the common Itanium / MSVC ABIs, but:
offsetofon a non-standard-layout type is conditionally-supported / undefined.So the whole introspection mechanism rests on assumptions the standard doesn't guarantee, and would break (silently, with wrong offsets) for records that aren't simple standard-layout aggregates.
Suggested direction
Document the constraint explicitly (reflectable records must be standard-layout, no inheritance) and enforce it at registration with a
static_assert, so a violating type fails to compile instead of misbehaving at runtime.Implementation note — the exact
static_assertneeds care (portability + C++11)Two corrections to the naive
static_assert(std::is_standard_layout_v<REFLECTABLE>):C++11: the project's minimum is C++11, so use
std::is_standard_layout<REFLECTABLE>::value(the
_valias is C++17).<type_traits>must be included.is_standard_layouton the record is implementation-dependent and may break CI. A class isstandard-layout only if all its non-static data members are themselves standard-layout — so
is_standard_layout<REFLECTABLE>is only true whenstd::wstring(everyMEMBER_TEXT) andTimePoint(MEMBER_DATETIME) are standard-layout, which varies by standard-libraryimplementation. Measured here: on libstdc++ (GCC/Clang on Linux)
is_standard_layout<std::wstring>and a record with
wstringmembers are bothtrue. libc++ (macOS) and MSVC (Windows) were NOTverifiable in this environment. If either makes
std::wstring/TimePointnon-standard-layout,asserting
is_standard_layout<REFLECTABLE>would fail to compile on that platform even forperfectly valid records — a CI regression, not a safety win. (Note
offsetofis already used onthese same types and works on all three CI compilers today, so the "UB" is theoretical for the
supported toolchains.)
Recommended enforcement
static_assert(!std::is_polymorphic<REFLECTABLE>::value, "reflectable records must not be polymorphic (no virtual functions/inheritance)");— this does notdepend on stdlib string layout (a struct with
wstringmembers is not polymorphic), and it catchesthe realistic footgun that actually breaks the pointer-to-member/offset hacks (a record made
virtual or given a vtable). Place it where the complete
REFLECTABLEtype is visible (e.g. insidethe generated
Register<REFLECTABLE>()ininclude/reflection.h).is_standard_layoutassert — only if verified: addstatic_assert(std::is_standard_layout<REFLECTABLE>::value, ...)ONLY after confirming it holds forthe test records on ALL THREE CI toolchains (Ubuntu/GCC, macOS/Clang, Windows/MSVC, C++11 and C++20).
If any platform fails it, do not add it — keep the polymorphic guard and document the constraint
instead, and leave this part as blocked (arguably needs the text representation to move off
std::wstring, cf. std::wstring + deprecated <codecvt> conversion is non-portable (BMP-only on Windows) #25).standard-layout members) in the README and/or the reflection macro header, so the requirement is
explicit regardless of which asserts are enforceable.