Skip to content

Commit

Permalink
refactor(reflection): no longer fallback to anonymous types automatic…
Browse files Browse the repository at this point in the history
…ally

This change was necessary due to the previous system being highly error prone.
Before, if reflect<T> was called for a type T without defined reflection included, a placeholder definition would be used which returned an anonymous type.
Sadly, due to the C++ One Definition Rule, this meant that other source files using reflect<T>, could return an anonymous type, even if the type's reflection definition was included!

TL;DR, previously a simple mistake such as forgetting to include an external reflection header could lead to a random reflection failure in the core of the library, such as a missing trait error.
Now, the users must define reflection for all types, as there is no longer a fallback. To simplify this for the previous 'anonymous types', a new CUBOS_ANONYMOUS_REFLECT macro was added.
  • Loading branch information
RiscadoA committed Aug 2, 2024
1 parent 350d910 commit 8e9a6d4
Show file tree
Hide file tree
Showing 39 changed files with 173 additions and 80 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Move Opt class to memory namespace (#1212, **@roby2014**).
- Moved from XPBD to TGS Soft for physics solving (#1269, **@fallenatlas**).
- Allow arbitrary input combinations for actions and axes (#417, #1279, **@luishfonseca**)
- Allow arbitrary input combinations for actions and axes (#417, #1279, **@luishfonseca**).
- Replaced fallback anonymous reflection types by a new anonymous reflection macro (#1163, **@RiscadoA**).

## [v0.2.0] - 2024-05-07

Expand Down
66 changes: 41 additions & 25 deletions core/include/cubos/core/reflection/reflect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,16 @@ namespace cubos::core::reflection
{
class Type;

/// @brief Creates a new unnamed type with the given identifier. Used as a fallback for non-reflectable C++ types.
///
/// The created 'unnamed' type is named from the given identifier, and has a minimal @ref ConstructibleTrait, which
/// exposes its identifier, size, alignment and destructor. This means that types without reflection which don't
/// expose destructors (e.g., singletons), are not supported by @ref reflect.
///
/// @param id Unique type identifier.
/// @brief Creates a new reflection type with a unique name created from a type name and a file path, and with a
/// minimal @ref ConstructibleTrait.
/// @param name Type name.
/// @param file Path of the file where the type is defined.
/// @param size Type size in bytes.
/// @param alignment Type alignment in bytes.
/// @param destructor Type destructor.
/// @ingroup core-reflection
CUBOS_CORE_API const Type& makeUnnamedType(unsigned long long id, std::size_t size, std::size_t alignment,
void (*destructor)(void*));
CUBOS_CORE_API const Type& makeAnonymousType(const char* name, const char* file, std::size_t size,
std::size_t alignment, void (*destructor)(void*));

/// @brief Defines the reflection function for the given type @p T.
///
Expand Down Expand Up @@ -62,25 +59,12 @@ namespace cubos::core::reflection
// your type or an external type.
static const Type& get()
{
constexpr bool ImplementsReflection = requires()
constexpr bool HasReflectionMethod = requires()
{
T::reflectGet();
};

if constexpr (ImplementsReflection)
{
return T::reflectGet();
}
else
{
// This variable is unused, but since there is one for each type, its address is
// guaranteed to be unique for each type. Thus, we use it as an identifier.
static const bool Var = false;
static const Type& type =
makeUnnamedType(reinterpret_cast<unsigned long long>(&Var), sizeof(T), alignof(T),
[](void* value) { static_cast<T*>(value)->~T(); });
return type;
}
static_assert(HasReflectionMethod, "Type does not implement reflection");
return T::reflectGet();
}
};

Expand Down Expand Up @@ -281,3 +265,35 @@ namespace cubos::core::reflection
\
template <CUBOS_PACK args> \
const ::cubos::core::reflection::Type& ::cubos::core::reflection::Reflect<CUBOS_PACK T>::make()

/// @brief Defines minimal reflection for a type private to a compilation unit.
///
/// Equivalent to @ref CUBOS_REFLECT with an implementation which uses @ref makeAnonymousType.
///
/// @code{.cpp}
/// // my_type.cpp
/// #include <cubos/core/reflection/reflect.hpp>
///
/// namespace
/// {
/// struct State
/// {
/// CUBOS_ANONYMOUS_REFLECT(State);
/// };
/// }
/// @endcode
///
/// @ingroup core-reflection
#define CUBOS_ANONYMOUS_REFLECT(...) \
static inline const cubos::core::reflection::Type& reflectGet() \
{ \
static const ::cubos::core::reflection::Type& type = reflectMake(); \
return type; \
} \
static inline const cubos::core::reflection::Type& reflectMake() \
{ \
return ::cubos::core::reflection::makeAnonymousType( \
#__VA_ARGS__, __FILE__, sizeof(__VA_ARGS__), alignof(__VA_ARGS__), \
[](void* ptr) { static_cast<__VA_ARGS__*>(ptr)->~__VA_ARGS__(); }); \
} \
static_assert(true, "") /* Here just to force the user to enter a semicolon */
4 changes: 3 additions & 1 deletion core/include/cubos/core/reflection/traits/inherits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ namespace cubos::core::reflection
/// @brief Provides inheritance relationship between types.
/// @see See @ref examples-core-reflection-traits-inherits for an example of using this trait.
/// @ingroup core-reflection
class InheritsTrait
class CUBOS_CORE_API InheritsTrait
{
public:
CUBOS_REFLECT;

~InheritsTrait() = default;

/// @brief Constructs.
Expand Down
12 changes: 0 additions & 12 deletions core/include/cubos/core/reflection/type.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ namespace cubos::core::reflection
/// @return Reference to the type.
static Type& create(std::string name);

/// @brief Constructs an unnamed type with the given identifier.
///
/// The type is also marked as not being implemented.
///
/// @param id Type identifier.
/// @return Reference to the type.
static Type& unnamed(unsigned long long id);

/// @brief Destroys the given type.
/// @param type Type to destroy.
static void destroy(Type& type);
Expand Down Expand Up @@ -122,9 +114,6 @@ namespace cubos::core::reflection
/// @return Whether the objects have the same address, which indicates equality.
bool operator==(const Type& other) const;

/// @brief Checks whether the type had reflection implemented for it.
bool implemented() const;

private:
~Type();

Expand All @@ -142,7 +131,6 @@ namespace cubos::core::reflection

std::string mName;
std::string mShortName;
bool mImplemented{true};
std::vector<Trait> mTraits;
};
} // namespace cubos::core::reflection
5 changes: 0 additions & 5 deletions core/samples/logging/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
#include <cubos/core/reflection/external/unordered_map.hpp>
/// [External reflection includes]

struct TypeWithoutReflection
{
};

int main()
{
/// [Set logging level]
Expand Down Expand Up @@ -42,6 +38,5 @@ int main()
CUBOS_INFO("An integer: {}", 1);
CUBOS_INFO("A glm::vec3: {}", glm::vec3(0.0F, 1.0F, 2.0F));
CUBOS_INFO("An std::unordered_map: {}", std::unordered_map<int, const char*>{{1, "one"}, {2, "two"}});
CUBOS_INFO("A type without reflection: {}", TypeWithoutReflection{});
}
/// [Logging macros with arguments]
6 changes: 6 additions & 0 deletions core/samples/reflection/basic/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,17 @@ struct Position
/// [Your own trait]
struct ColorTrait
{
CUBOS_REFLECT;
float r, g, b;
};
/// [Your own trait]

/// [Adding your own trait]
CUBOS_REFLECT_IMPL(ColorTrait)
{
return Type::create("ColorTrait");
}

CUBOS_REFLECT_IMPL(Position)
{
return Type::create("Position").with(ColorTrait{.r = 0.0F, .g = 1.0F, .b = 0.0F});
Expand Down
1 change: 1 addition & 0 deletions core/src/ecs/dynamic.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <cubos/core/ecs/dynamic.hpp>
#include <cubos/core/log.hpp>
#include <cubos/core/reflection/external/cstring.hpp>
#include <cubos/core/reflection/external/primitives.hpp>
#include <cubos/core/reflection/external/string.hpp>

Expand Down
12 changes: 1 addition & 11 deletions core/src/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,17 +243,7 @@ const char* Logger::streamFormat(memory::Stream& stream, const char* format, con
}

foundArgument = true;
if (!type.implemented())
{
CUBOS_WARN("You tried to print a type ({}) which doesn't implement reflection. Did you forget to "
"include its reflection definition?",
type.name());
stream.print("(no reflection)");
}
else
{
data::DebugSerializer{stream}.write(type, value);
}
data::DebugSerializer{stream}.write(type, value);
++format;
}
else
Expand Down
47 changes: 44 additions & 3 deletions core/src/reflection/reflect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,49 @@
#include <cubos/core/reflection/traits/constructible.hpp>
#include <cubos/core/reflection/type.hpp>

auto cubos::core::reflection::makeUnnamedType(unsigned long long id, std::size_t size, std::size_t alignment,
void (*destructor)(void*)) -> const Type&
auto cubos::core::reflection::makeAnonymousType(const char* name, const char* file, std::size_t size,
std::size_t alignment, void (*destructor)(void*)) -> const Type&
{
return Type::unnamed(id).with(ConstructibleTrait{size, alignment, destructor});
// Replace non-ASCII-alphanumeric characters in the file path with underscores.
std::string fileName;
for (const char* c = file; *c != '\0'; ++c)
{
if ((*c >= 'a' && *c <= 'z') || (*c >= 'A' && *c <= 'Z') || (*c >= '0' && *c <= '9'))
{
fileName.push_back(*c);
}
else
{
fileName.push_back('_');
}
}

// Remove the "_src_" prefix from the file name.
if (auto cursor = fileName.find("_src_"); cursor != std::string::npos)
{
fileName.erase(0, cursor + 5);
}

// The "_include_" too.
if (auto cursor = fileName.find("_include_"); cursor != std::string::npos)
{
fileName.erase(0, cursor + 9);
}

// Remove the "_cpp" suffix from the file name.
if (fileName.ends_with("_cpp"))
{
fileName.erase(fileName.size() - 4, 4);
}

// And the "_hpp" suffix too.
if (fileName.ends_with("_hpp"))
{
fileName.erase(fileName.size() - 4, 4);
}

std::string fullName = name;
fullName.append("#");
fullName.append(fileName);
return Type::create(fullName).with(ConstructibleTrait{size, alignment, destructor});
}
5 changes: 5 additions & 0 deletions core/src/reflection/traits/inherits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@

using namespace cubos::core::reflection;

CUBOS_REFLECT_IMPL(InheritsTrait)
{
return Type::create("cubos::core::ecs::InheritsTrait");
}

const Type& InheritsTrait::base()
{
return *mType;
Expand Down
12 changes: 0 additions & 12 deletions core/src/reflection/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ Type& Type::create(std::string name)
return *(new Type(std::move(name)));
}

Type& Type::unnamed(unsigned long long id)
{
auto& type = Type::create("unnamed" + std::to_string(id));
type.mImplemented = false;
return type;
}

void Type::destroy(Type& type)
{
delete &type;
Expand Down Expand Up @@ -81,11 +74,6 @@ bool Type::operator==(const Type& other) const
return false;
}

bool Type::implemented() const
{
return mImplemented;
}

Type::~Type()
{
for (auto& trait : mTraits)
Expand Down
17 changes: 7 additions & 10 deletions core/tests/reflection/reflect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,27 @@ CUBOS_REFLECT_EXTERNAL_TEMPLATE((typename T), (Templated<T>))
return Type::create("Templated<" + reflect<T>().name() + ">");
}

/// @brief Type without reflection.
struct Unreflected
/// @brief Type with anonymous reflection.
struct Anonymous
{
CUBOS_ANONYMOUS_REFLECT(Anonymous);
};

TEST_CASE("reflection::reflect")
{
CHECK(reflect<Internal>().implemented());
CHECK(reflect<Internal>().name() == "Internal");
CHECK(reflect<Internal>().is<Internal>());

CHECK(reflect<External>().implemented());
CHECK(reflect<External>().name() == "External");
CHECK(reflect<External>().is<External>());
CHECK_FALSE(reflect<External>().is<Internal>());

CHECK(reflect<Templated<Internal>>().implemented());
CHECK(reflect<Templated<Internal>>().name() == "Templated<Internal>");
CHECK(reflect<Templated<External>>().name() == "Templated<External>");
CHECK(reflect<Templated<Templated<Internal>>>().name() == "Templated<Templated<Internal>>");

CHECK_FALSE(reflect<Unreflected>().implemented());
CHECK(reflect<Unreflected>().is<Unreflected>());
CHECK(reflect<Unreflected>().has<ConstructibleTrait>());
CHECK(reflect<Unreflected>().get<ConstructibleTrait>().size() == sizeof(Unreflected));
CHECK(reflect<Unreflected>().get<ConstructibleTrait>().alignment() == alignof(Unreflected));
CHECK(reflect<Anonymous>().is<Anonymous>());
CHECK(reflect<Anonymous>().has<ConstructibleTrait>());
CHECK(reflect<Anonymous>().get<ConstructibleTrait>().size() == sizeof(Anonymous));
CHECK(reflect<Anonymous>().get<ConstructibleTrait>().alignment() == alignof(Anonymous));
}
2 changes: 2 additions & 0 deletions engine/samples/collisions/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ static CUBOS_DEFINE_TAG(collisionsSampleUpdated);

struct State
{
CUBOS_ANONYMOUS_REFLECT(State);

bool collided = false;

Entity a;
Expand Down
2 changes: 2 additions & 0 deletions engine/samples/complex_physics/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ static const glm::vec3 AimPoint = glm::vec3(0.0F, 3.0F, 0.0F);

struct TimeBetweenShoots
{
CUBOS_ANONYMOUS_REFLECT(TimeBetweenShoots);

float max = 3.0F;
float current = 0.0F;
};
Expand Down
4 changes: 4 additions & 0 deletions engine/samples/events/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@ CUBOS_DEFINE_TAG(cubos::engine::eventC);
/// [Event struct]
struct MyEvent
{
CUBOS_ANONYMOUS_REFLECT(MyEvent);

int value;
};
/// [Event struct]

struct State
{
CUBOS_ANONYMOUS_REFLECT(State);

int step;
};

Expand Down
2 changes: 2 additions & 0 deletions engine/samples/input/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ static const Asset<InputBindings> BindingsAsset = AnyAsset("bf49ba61-5103-41bc-9

struct State
{
CUBOS_ANONYMOUS_REFLECT(State);

int showcase = 0;
bool nextPressed = false;
bool explained = false;
Expand Down
2 changes: 2 additions & 0 deletions engine/samples/physics/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ static const Asset<VoxelPalette> PaletteAsset = AnyAsset("1aa5e234-28cb-4386-99b

struct MaxTime
{
CUBOS_ANONYMOUS_REFLECT(MaxTime);

float max = 1.0F;
float current = 0.0F;
};
Expand Down
2 changes: 2 additions & 0 deletions engine/src/fixed_step/plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ namespace
{
struct AccumulatedTime
{
CUBOS_ANONYMOUS_REFLECT(AccumulatedTime);

float value = 0.0F;
};
} // namespace
Expand Down
Loading

0 comments on commit 8e9a6d4

Please sign in to comment.