From 09530e15c917da62c2c2a558918ee696dad15076 Mon Sep 17 00:00:00 2001 From: King_DuckZ Date: Fri, 3 Jun 2022 09:39:43 +0200 Subject: [PATCH] Use a better ID for identifying classes. Upon testing (and reading), getting the address of a static is safe even across .so boundaries. Moreover it doesn't incur into the risk of collisions, unlike hashes, and also works with different classes with the same name (for example from two different unnamed namespaces). At this point the crc32 stuff is pretty much redundant, I'll try to remove it next. --- include/wrenpp/class_manager.hpp | 59 +++++++-------- include/wrenpp/detail/has_method.hpp | 12 ++++ include/wrenpp/detail/meson.build | 1 + include/wrenpp/detail/module_and_name.hpp | 24 +++++-- include/wrenpp/detail/type_id.hpp | 71 +++++++++++++++++++ .../detail/wren_class_name_from_type.hpp | 15 ++-- include/wrenpp/vm.hpp | 22 +++--- src/class_manager.cpp | 15 +++- src/vm.cpp | 13 ++-- src/wren_class_name_from_type.cpp | 2 +- 10 files changed, 164 insertions(+), 70 deletions(-) create mode 100644 include/wrenpp/detail/type_id.hpp diff --git a/include/wrenpp/class_manager.hpp b/include/wrenpp/class_manager.hpp index 42be406..cfbfd60 100644 --- a/include/wrenpp/class_manager.hpp +++ b/include/wrenpp/class_manager.hpp @@ -21,6 +21,7 @@ #include "detail/strings_in_vector.hpp" #include "detail/construct_foreign_class.hpp" #include "detail/wren_class_name_from_type.hpp" +#include "detail/type_id.hpp" #include #include #include @@ -79,22 +80,28 @@ namespace wren { std::size_t operator()(TempClassName value) const; }; - template - struct CppClassTypeHelper { - static void register_ifp(ClassManager&, const ClassNameOwning* name); + class TypeIDHash { + public: + std::size_t operator()(TypeID tid) const; }; - template - struct CppClassTypeHelper> { - static void register_ifp(ClassManager& classman, const ClassNameOwning* name); - }; + define_has_typedef(class_type, ClassType); } //namespace detail + //Customisation point - specialise for your own types if needed + template + struct ClassMakerTraits { + }; + + template + struct ClassMakerTraits> { + typedef T class_type; + }; + class ClassManager { - template friend struct detail::CppClassTypeHelper; typedef std::function make_foreign_class_t; typedef std::unordered_map ClassMap; - typedef std::unordered_map TypeMap; + typedef std::unordered_map TypeMap; public: ClassManager(); ~ClassManager() noexcept; @@ -102,7 +109,7 @@ namespace wren { template ClassManager& add_class_maker (std::string module_name, std::string_view class_name, F&& maker); foreign_class_t make_class(std::string_view module_name, std::string_view class_name); - ModuleAndName wren_class_name_from_hash(ClassManagerNameHashType hash) const; + ModuleAndName wren_class_name_from_hash(TypeID hash) const; template ModuleAndName wren_class_name_from_type() const; @@ -113,33 +120,27 @@ namespace wren { make_foreign_class_t ); - void add_type (ClassManagerNameHashType hash, const detail::ClassNameOwning* name); + void add_type (TypeID hash, const detail::ClassNameOwning* name); ClassMap m_classes; TypeMap m_types; //refers to memory in m_classes }; - namespace detail { - template - inline void CppClassTypeHelper::register_ifp(ClassManager&, const ClassNameOwning*) { - //Nothing to do - } - - template - inline void CppClassTypeHelper>::register_ifp( - ClassManager& classman, - const ClassNameOwning* name - ) { - const auto new_hash = type_hash(); - classman.add_type(new_hash, name); - } - } //namespace detail - template inline ClassManager& ClassManager::add_class_maker (std::string module_name, std::string_view class_name, F&& maker) { + typedef ClassMakerTraits> MakerTraits; + auto it_new = this->add_class_maker_implem(module_name, class_name, std::forward(maker)); - const auto& owning_name = it_new->first; - detail::CppClassTypeHelper>::register_ifp(*this, &owning_name); + + //See if we can deduce the class type that maker is supposed to work + //with. This is not guaranteed to be an existing piece of info because + //user code can provide their own function that doesn't use any actual + //c++ class, just normal functions. + if constexpr (detail::HasClassTypeTypedef::value) { + const auto& owning_name = it_new->first; + this->add_type(type_id(), &owning_name); + } + return *this; } diff --git a/include/wrenpp/detail/has_method.hpp b/include/wrenpp/detail/has_method.hpp index 0c9e430..14e22dd 100644 --- a/include/wrenpp/detail/has_method.hpp +++ b/include/wrenpp/detail/has_method.hpp @@ -66,5 +66,17 @@ }; \ } +#define define_has_typedef(typedef_name,pretty_name) \ + template \ + struct Has ## pretty_name ## Typedef { \ + private: \ + struct TrueType { int a[2]; }; \ + typedef int FalseType; \ + template static TrueType has_typedef ( const typename C::typedef_name* ); \ + template static FalseType has_typedef ( ... ); \ + public: \ + enum { value = sizeof(has_typedef(nullptr)) == sizeof(TrueType) }; \ + } + namespace wren { } //namespace wren diff --git a/include/wrenpp/detail/meson.build b/include/wrenpp/detail/meson.build index 5e7f344..2cbd177 100644 --- a/include/wrenpp/detail/meson.build +++ b/include/wrenpp/detail/meson.build @@ -8,6 +8,7 @@ include_files = [ 'setters_getters.hpp', 'string_bt.hpp', 'strings_in_vector.hpp', + 'type_id.hpp', 'wren_class_name_from_type.hpp', 'wren_types.hpp', ] diff --git a/include/wrenpp/detail/module_and_name.hpp b/include/wrenpp/detail/module_and_name.hpp index 378e5f9..1db2bfe 100644 --- a/include/wrenpp/detail/module_and_name.hpp +++ b/include/wrenpp/detail/module_and_name.hpp @@ -25,6 +25,7 @@ #include namespace wren { + typedef decltype(crc32c(nullptr, 0)) ModuleAndNameHashType; class ModuleAndName; namespace detail { @@ -37,8 +38,10 @@ namespace wren { bool deep_equal (const ModuleAndName& a, const ModuleAndName& b) noexcept; } //unnamed namespace - //Non-owning, lightweight class for storing a module nome/class name pair. - //They are stored as consecutive strings within a single buffer with a + //Non-owning, lightweight class for storing a module name + generic name + //pair. It holds a hash value too, which is currently only meaningful for + //module name/class name pairs that are mapped to a c++ class. + //Names are stored as consecutive strings within a single buffer with a //null-char in-between and at the end so that each name is null-terminated //and can be used with C APIs. //Keep the size small, ideally within 2 registers size. Altering this might @@ -51,7 +54,7 @@ namespace wren { friend bool detail::deep_equal(const ModuleAndName&, const ModuleAndName&) noexcept; public: constexpr ModuleAndName(); - constexpr ModuleAndName (const char* base, std::uint32_t hash, std::size_t mod_name_len, std::size_t class_name_len); + constexpr ModuleAndName (const char* base, ModuleAndNameHashType hash, std::size_t mod_name_len, std::size_t class_name_len); constexpr bool operator==(const ModuleAndName& other) const noexcept; constexpr bool operator!=(const ModuleAndName& other) const noexcept; @@ -64,14 +67,20 @@ namespace wren { constexpr std::string_view module_name() const noexcept { return {module_name_char(), m_mod_name_len}; } constexpr std::string_view class_name() const noexcept { return {class_name_char(), m_class_name_len}; } constexpr const char* buffer_address() const noexcept { return m_base; } + constexpr ModuleAndNameHashType hash() const noexcept { return m_hash; } private: constexpr std::size_t raw_buffer_size() const noexcept; const char* m_base; - std::uint32_t m_hash; + ModuleAndNameHashType m_hash; std::uint16_t m_mod_name_len; std::uint16_t m_class_name_len; + + static_assert( + sizeof(m_hash) <= sizeof(std::uint32_t), + "Hash type is too large, review this struct or change it back to uint32_t" + ); }; union RawAccessModuleAndName { @@ -93,7 +102,7 @@ namespace wren { ModuleAndName(nullptr, 0, 0, 0) {} - constexpr ModuleAndName::ModuleAndName (const char* base, std::uint32_t hash, std::size_t mod_name_len, std::size_t class_name_len) : + constexpr ModuleAndName::ModuleAndName (const char* base, ModuleAndNameHashType hash, std::size_t mod_name_len, std::size_t class_name_len) : m_base(base), m_hash(hash), m_mod_name_len(mod_name_len), @@ -137,7 +146,10 @@ namespace wren { constexpr const char* data = StaticStorage::value.data(); constexpr std::uint16_t s1_len = static_cast(S1.size()); constexpr std::uint16_t s2_len = static_cast(S2.size()); - constexpr std::uint32_t hash = crc32c(data, StaticStorage::value.size()); + constexpr ModuleAndNameHashType hash = crc32c( + StaticStorage::value.data(), + StaticStorage::value.size() + ); return ModuleAndName{data, hash, s1_len, s2_len}; } diff --git a/include/wrenpp/detail/type_id.hpp b/include/wrenpp/detail/type_id.hpp new file mode 100644 index 0000000..591ebc1 --- /dev/null +++ b/include/wrenpp/detail/type_id.hpp @@ -0,0 +1,71 @@ +/* Copyright 2020-2022, Michele Santullo + * This file is part of wrenpp. + * + * Wrenpp is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Wrenpp is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wrenpp. If not, see . + */ + +#pragma once + +#include + +namespace wren { + //template + //[[gnu::const]] + //inline constexpr std::uint32_t type_hash() { + // return crc32c(__PRETTY_FUNCTION__); + //} + + //from: + //https://codereview.stackexchange.com/questions/48594/unique-type-id-no-rtti + class TypeID { + using sig = TypeID(); + + sig* m_id; + constexpr TypeID(sig* id) : m_id{id} {} + + public: + template + friend constexpr TypeID type_id(); + + TypeID (const TypeID&) = default; + TypeID (TypeID&&) = default; + ~TypeID() noexcept = default; + TypeID& operator= (const TypeID&) = default; + TypeID& operator= (TypeID&&) = default; + + constexpr bool operator==(TypeID other) const noexcept; + constexpr bool operator!=(TypeID other) const noexcept; + constexpr bool operator<(TypeID other) const noexcept; + constexpr std::uintptr_t id() const noexcept; + }; + + template + constexpr TypeID type_id() { return &type_id; } + + constexpr bool TypeID::operator==(TypeID other) const noexcept { + return id() == other.id(); + } + + constexpr bool TypeID::operator!=(TypeID other) const noexcept { + return id() != other.id(); + } + + constexpr bool TypeID::operator<(TypeID other) const noexcept { + return id() < other.id(); + } + + constexpr std::uintptr_t TypeID::id() const noexcept { + return reinterpret_cast(m_id); + } +} //namespace wren diff --git a/include/wrenpp/detail/wren_class_name_from_type.hpp b/include/wrenpp/detail/wren_class_name_from_type.hpp index d0d8869..5ca0186 100644 --- a/include/wrenpp/detail/wren_class_name_from_type.hpp +++ b/include/wrenpp/detail/wren_class_name_from_type.hpp @@ -18,34 +18,27 @@ #pragma once #include "module_and_name.hpp" +#include "type_id.hpp" +#include "crc32.hpp" #include #include -#include namespace wren { - typedef std::size_t ClassManagerNameHashType; class VM; namespace detail { - //TODO: work in progress, make it take a type T and hash that for real - template - inline ClassManagerNameHashType type_hash() { - static const ClassManagerNameHashType my_hash = 0; - return static_cast(reinterpret_cast(&my_hash)); - } - //Just an utility function to call a method on the class manager in vm //and return its return value without including the header for the //class manager. Useful in inline functions in header files where //including class_manager.hpp would propagate it everywhere. ModuleAndName fetch_class_name_from_manager ( const VM& vm, - ClassManagerNameHashType hash + TypeID hash ); template inline ModuleAndName wren_class_name_from_type (const VM& vm) { - return fetch_class_name_from_manager(vm, type_hash()); + return fetch_class_name_from_manager(vm, type_id()); } } //namespace detail } //namespace wren diff --git a/include/wrenpp/vm.hpp b/include/wrenpp/vm.hpp index dbe6221..c4050ce 100644 --- a/include/wrenpp/vm.hpp +++ b/include/wrenpp/vm.hpp @@ -19,7 +19,7 @@ #include "detail/has_method.hpp" #include "detail/error_type.hpp" -#include "detail/crc32.hpp" +#include "detail/type_id.hpp" #include "detail/wren_types.hpp" #include "handle.hpp" #include @@ -116,11 +116,11 @@ namespace wren { private: struct LocalData; - VM (Configuration* conf, const detail::Callbacks&, void* user_data, std::uint32_t user_data_type); + VM (Configuration* conf, const detail::Callbacks&, void* user_data, TypeID user_data_type); DynafuncMaker* dynafunc_maker(); - std::uint32_t user_data_type() const; + TypeID user_data_type() const; void* void_user_data(); - void set_user_data (void* user_data, std::uint32_t user_data_type); + void set_user_data (void* user_data, TypeID user_data_type); template detail::Callbacks to_callbacks (T& conf); std::unique_ptr m_local; @@ -151,12 +151,6 @@ namespace wren { return (*M)(args...); } }; - - template - [[gnu::const]] - inline consteval std::uint32_t type_id() { - return crc32c(__PRETTY_FUNCTION__); - } } //namespace detail template @@ -198,7 +192,7 @@ namespace wren { template inline U* VM::user_data() { - if (user_data_type() != detail::type_id()) { + if (user_data_type() != type_id()) { throw std::runtime_error("Wrong user data type requested"); } return reinterpret_cast(void_user_data()); @@ -206,7 +200,7 @@ namespace wren { template inline void VM::set_user_data (U* user_data) { - set_user_data(reinterpret_cast(user_data), detail::type_id()); + set_user_data(reinterpret_cast(user_data), type_id()); } template @@ -215,7 +209,7 @@ namespace wren { static_cast(conf), to_callbacks(*conf), nullptr, - detail::type_id() + type_id() ) { } @@ -226,7 +220,7 @@ namespace wren { static_cast(conf), to_callbacks(*conf), reinterpret_cast(user_data), - detail::type_id() + type_id() ) { } diff --git a/src/class_manager.cpp b/src/class_manager.cpp index 0756f5c..ae5d0ca 100644 --- a/src/class_manager.cpp +++ b/src/class_manager.cpp @@ -80,6 +80,15 @@ namespace wren { std::size_t ClassNameHash::operator()(TempClassName value) const { return value.module_hash() ^ (value.class_hash() << 1); } + + std::size_t TypeIDHash::operator()(TypeID tid) const { + if constexpr (sizeof(tid.id()) > sizeof(std::size_t)) { + return std::hash{}(tid.id()); + } + else { + return static_cast(tid.id()); + } + } } //namespace detail ClassManager::ClassManager() = default; @@ -119,15 +128,15 @@ namespace wren { return retval; } - void ClassManager::add_type (ClassManagerNameHashType hash, const detail::ClassNameOwning* name) { + void ClassManager::add_type (TypeID hash, const detail::ClassNameOwning* name) { using detail::TempClassName; - assert(m_types.count(hash) == 0 or m_types.at(hash) == name); + assert(m_types.count(hash) == 0 or *m_types.at(hash) == *name); //insert if not present, leave old value if hash is already there m_types.insert(std::make_pair(hash, name)); } - ModuleAndName ClassManager::wren_class_name_from_hash(ClassManagerNameHashType hash) const { + ModuleAndName ClassManager::wren_class_name_from_hash(TypeID hash) const { const auto it_found = m_types.find(hash); if (m_types.cend() != it_found) { const detail::ClassNameOwning* const name = it_found->second; diff --git a/src/vm.cpp b/src/vm.cpp index f6173d0..aadc148 100644 --- a/src/vm.cpp +++ b/src/vm.cpp @@ -162,7 +162,8 @@ namespace wren { struct VM::LocalData { explicit LocalData (const detail::Callbacks& cb) : callbacks(cb), - wvm(nullptr) + wvm(nullptr), + user_data_type(type_id()) { callbacks.dynafunc = &dynafunc; } @@ -180,10 +181,10 @@ namespace wren { DynafuncMaker dynafunc; WrenVM* wvm; void* user_data; - std::uint32_t user_data_type; + TypeID user_data_type; }; - VM::VM (Configuration* conf, const detail::Callbacks& cb, void* user_data, std::uint32_t user_data_type) : + VM::VM (Configuration* conf, const detail::Callbacks& cb, void* user_data, TypeID user_data_type) : m_local(std::make_unique(cb)) { this->reset(conf); @@ -197,7 +198,7 @@ namespace wren { return &m_local->dynafunc; } - std::uint32_t VM::user_data_type() const { + TypeID VM::user_data_type() const { return m_local->user_data_type; } @@ -205,7 +206,7 @@ namespace wren { return m_local->user_data; } - void VM::set_user_data (void* user_data, std::uint32_t user_data_type) { + void VM::set_user_data (void* user_data, TypeID user_data_type) { m_local->user_data = user_data; m_local->user_data_type = user_data_type; } @@ -281,7 +282,7 @@ namespace wren { void VM::set_user_data (std::nullptr_t) { m_local->user_data = nullptr; - m_local->user_data_type = detail::type_id(); + m_local->user_data_type = type_id(); } bool VM::has_user_data() const noexcept { diff --git a/src/wren_class_name_from_type.cpp b/src/wren_class_name_from_type.cpp index a0cdb66..84f23ab 100644 --- a/src/wren_class_name_from_type.cpp +++ b/src/wren_class_name_from_type.cpp @@ -6,7 +6,7 @@ namespace wren { namespace detail { ModuleAndName fetch_class_name_from_manager ( const VM& vm, - ClassManagerNameHashType hash + TypeID hash ) { return vm.class_manager().wren_class_name_from_hash(hash); }