diff --git a/README.md b/README.md index c450e47..cfca1bc 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,14 @@ Supported field macros: | `sqlite_reflection::TimePoint` | `MEMBER_DATETIME(name)` | | member function declaration | `FUNC(signature)` | +**Layout constraint.** Reflectable records must be simple, standard-layout structs: no base +classes, no virtual functions, no virtual/multiple inheritance. Member access is computed from +`offsetof`/pointer-to-member byte offsets, which are only well-defined for such types; a struct +outside these bounds is rejected at compile time via a `static_assert` if it's polymorphic (has a +vtable), but other standard-layout violations are not otherwise detectable across all supported +compilers and would silently compute wrong member offsets instead of failing to compile. Stick to +plain data members declared through the `MEMBER_*` macros and you're always within these bounds. + Make sure each reflected record header is included by your program before `Database::Initialize()` is called. During initialization, the library creates one table for each registered record type if that table does not already exist. ## Opening and closing the database diff --git a/include/reflection.h b/include/reflection.h index 6b046de..cf12e24 100644 --- a/include/reflection.h +++ b/include/reflection.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -153,6 +154,11 @@ REFLECTION_EXPORT char* GetMemberAddress(void* p, const Reflection& record, size #pragma warning(push) #pragma warning(disable : 4002) // "too many actual parameters for macro 'MEMBER'" +/// Reflectable records must be simple, standard-layout structs: no base classes, no virtual +/// functions, no virtual inheritance. Member access is computed via offsetof/pointer-to-member +/// byte offsets (see OffsetFromStart and DEFINE_MEMBER above), which only give correct answers +/// for such types; a struct outside these bounds gets WRONG member offsets silently (data +/// corruption, not a compile or runtime error) unless caught by the static_assert below. struct REFLECTABLE_DLL_EXPORT REFLECTABLE { // member declaration according to the order given in source code #define MEMBER_DECLARE(L, R) L R; @@ -188,6 +194,24 @@ struct REFLECTABLE_DLL_EXPORT REFLECTABLE { #undef FUNC }; +// Reject the realistic footgun that actually breaks OffsetFromStart's pointer-to-member byte +// hack and offsetof's standard-layout requirement: giving a record a vtable via a virtual +// function or virtual/multiple inheritance. This does not depend on standard-library string +// layout (a struct with wstring members is never polymorphic on its own), so it holds on every +// supported compiler/platform. +static_assert(!std::is_polymorphic::value, + "sqlite-reflection: reflectable records must not be polymorphic " + "(no virtual functions or virtual/multiple inheritance)."); + +// A stronger static_assert(std::is_standard_layout::value, ...) was attempted here +// and confirmed via CI to fail to compile on MSVC (Windows), in both C++11 and C++20, for the +// existing test records - std::wstring and/or sqlite_reflection::TimePoint are not +// standard-layout on that standard library. It compiled fine on GCC/libstdc++ (Linux) and +// Clang/libc++ (macOS). Since is_standard_layout is implementation-defined and this project +// supports MSVC, that guard is not enforceable portably today; it's deferred pending #25 (moving +// the text representation off std::wstring). The polymorphic guard above remains the enforced, +// portable constraint. + /// Provide a static registration function for each reflectable struct static std::string CAT(Register, REFLECTABLE)() { std::string type_id = typeid(REFLECTABLE).name(); diff --git a/tests/reflection_test.cc b/tests/reflection_test.cc new file mode 100644 index 0000000..55ff139 --- /dev/null +++ b/tests/reflection_test.cc @@ -0,0 +1,59 @@ +// MIT License +// +// Copyright (c) 2026 Ioannis Kaliakatsos +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +#include "reflection.h" + +#include + +#include + +#include "company.h" +#include "datetime_container.h" +#include "person.h" +#include "pet.h" + +using namespace sqlite_reflection; + +// Reflectable records must not be polymorphic (see include/reflection.h's static_assert next to +// the REFLECTABLE struct definition). These compile-time checks pin that guarantee for every +// storage class the test records exercise (TEXT/wstring, INT, REAL, BOOL, DATETIME/TimePoint), +// so a regression that reintroduces a virtual function/inheritance into the macro-generated +// struct fails the build here rather than silently corrupting member offsets at runtime. +static_assert(!std::is_polymorphic::value, "Person must not be polymorphic"); +static_assert(!std::is_polymorphic::value, "Pet must not be polymorphic"); +static_assert(!std::is_polymorphic::value, "Company must not be polymorphic"); +static_assert(!std::is_polymorphic::value, "DatetimeContainer must not be polymorphic"); + +// Mirrors the static_asserts above as ordinary runtime expectations, so the guarantee is also +// visible in normal test output rather than only enforced silently at compile time. +// +// A stronger ReflectableRecordsAreStandardLayout test (mirroring +// static_assert(std::is_standard_layout::value, ...)) was attempted alongside this +// one and confirmed via CI to fail on MSVC (Windows), in both C++11 and C++20 - std::wstring +// and/or TimePoint are not standard-layout on that standard library - so it was removed; see the +// comment in include/reflection.h next to the REFLECTABLE struct definition. +TEST(ReflectionTest, ReflectableRecordsAreNotPolymorphic) { + EXPECT_FALSE(std::is_polymorphic::value); + EXPECT_FALSE(std::is_polymorphic::value); + EXPECT_FALSE(std::is_polymorphic::value); + EXPECT_FALSE(std::is_polymorphic::value); +}