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

@@ -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 <unordered_map>
 #include <string>
 #include <type_traits>
@@ -79,22 +80,28 @@ namespace wren {
 			std::size_t operator()(TempClassName value) const;
 		};
 
-		template <typename F>
-		struct CppClassTypeHelper {
-			static void register_ifp(ClassManager&, const ClassNameOwning* name);
+		class TypeIDHash {
+		public:
+			std::size_t operator()(TypeID tid) const;
 		};
 
-		template <typename T, typename... Args>
-		struct CppClassTypeHelper<MakeForeignClass<T, Args...>> {
-			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 <typename T>
+	struct ClassMakerTraits {
+	};
+
+	template <typename T, typename... Args>
+	struct ClassMakerTraits<detail::MakeForeignClass<T, Args...>> {
+		typedef T class_type;
+	};
+
 	class ClassManager {
-		template <typename F> friend struct detail::CppClassTypeHelper;
 		typedef std::function<foreign_class_t()> make_foreign_class_t;
 		typedef std::unordered_map<detail::ClassNameOwning, make_foreign_class_t, detail::ClassNameHash, detail::ClassNameEqual> ClassMap;
-		typedef std::unordered_map<ClassManagerNameHashType, const detail::ClassNameOwning*> TypeMap;
+		typedef std::unordered_map<TypeID, const detail::ClassNameOwning*, detail::TypeIDHash> TypeMap;
 	public:
 		ClassManager();
 		~ClassManager() noexcept;
@@ -102,7 +109,7 @@ namespace wren {
 		template <typename F>
 		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 <typename T>
 		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 <typename F>
-		inline void CppClassTypeHelper<F>::register_ifp(ClassManager&, const ClassNameOwning*) {
-			//Nothing to do
-		}
-
-		template <typename T, typename... Args>
-		inline void CppClassTypeHelper<MakeForeignClass<T, Args...>>::register_ifp(
-			ClassManager& classman,
-			const ClassNameOwning* name
-		) {
-			const auto new_hash = type_hash<T>();
-			classman.add_type(new_hash, name);
-		}
-	} //namespace detail
-
 	template <typename F>
 	inline ClassManager& ClassManager::add_class_maker (std::string module_name, std::string_view class_name, F&& maker) {
+		typedef ClassMakerTraits<std::decay_t<F>> MakerTraits;
+
 		auto it_new = this->add_class_maker_implem(module_name, class_name, std::forward<F>(maker));
-		const auto& owning_name = it_new->first;
-		detail::CppClassTypeHelper<std::decay_t<F>>::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<MakerTraits>::value) {
+			const auto& owning_name = it_new->first;
+			this->add_type(type_id<typename MakerTraits::class_type>(), &owning_name);
+		}
+
 		return *this;
 	}
 

include/wrenpp/detail/has_method.hpp

@@ -66,5 +66,17 @@
 		}; \
 	}
 
+#define define_has_typedef(typedef_name,pretty_name) \
+	template <typename T> \
+	struct Has ## pretty_name ## Typedef { \
+	private: \
+		struct TrueType { int a[2]; }; \
+		typedef int FalseType; \
+		template <typename C> static TrueType has_typedef ( const typename C::typedef_name* ); \
+		template <typename C> static FalseType has_typedef ( ... ); \
+	public: \
+		enum { value = sizeof(has_typedef<T>(nullptr)) == sizeof(TrueType) }; \
+	}
+
 namespace wren {
 } //namespace wren

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',
 ]

include/wrenpp/detail/module_and_name.hpp

@@ -25,6 +25,7 @@
 #include <type_traits>
 
 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<std::uint16_t>(S1.size());
 		constexpr std::uint16_t s2_len = static_cast<std::uint16_t>(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};
 	}

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 <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <cstdint>
+
+namespace wren {
+	//template <typename U>
+	//[[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<typename T>
+		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<typename T>
+	constexpr TypeID type_id() { return &type_id<T>; }
+
+	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<std::uintptr_t>(m_id);
+	}
+} //namespace wren

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 <utility>
 #include <string_view>
-#include <cstddef>
 
 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<typename T>
-		inline ClassManagerNameHashType type_hash() {
-			static const ClassManagerNameHashType my_hash = 0;
-			return static_cast<ClassManagerNameHashType>(reinterpret_cast<std::uintptr_t>(&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 <typename T>
 		inline ModuleAndName wren_class_name_from_type (const VM& vm) {
-			return fetch_class_name_from_manager(vm, type_hash<T>());
+			return fetch_class_name_from_manager(vm, type_id<T>());
 		}
 	} //namespace detail
 } //namespace wren

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 <memory>
@@ -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 <typename T> detail::Callbacks to_callbacks (T& conf);
 
 		std::unique_ptr<LocalData> m_local;
@@ -151,12 +151,6 @@ namespace wren {
 				return (*M)(args...);
 			}
 		};
-
-		template <typename U>
-		[[gnu::const]]
-		inline consteval std::uint32_t type_id() {
-			return crc32c(__PRETTY_FUNCTION__);
-		}
 	} //namespace detail
 
 	template <typename T>
@@ -198,7 +192,7 @@ namespace wren {
 
 	template <typename U>
 	inline U* VM::user_data() {
-		if (user_data_type() != detail::type_id<U>()) {
+		if (user_data_type() != type_id<U>()) {
 			throw std::runtime_error("Wrong user data type requested");
 		}
 		return reinterpret_cast<U*>(void_user_data());
@@ -206,7 +200,7 @@ namespace wren {
 
 	template <typename U>
 	inline void VM::set_user_data (U* user_data) {
-		set_user_data(reinterpret_cast<void*>(user_data), detail::type_id<U>());
+		set_user_data(reinterpret_cast<void*>(user_data), type_id<U>());
 	}
 
 	template <typename T>
@@ -215,7 +209,7 @@ namespace wren {
 			static_cast<Configuration*>(conf),
 			to_callbacks(*conf),
 			nullptr,
-			detail::type_id<std::nullptr_t>()
+			type_id<std::nullptr_t>()
 		)
 	{
 	}
@@ -226,7 +220,7 @@ namespace wren {
 			static_cast<Configuration*>(conf),
 			to_callbacks(*conf),
 			reinterpret_cast<void*>(user_data),
-			detail::type_id<U>()
+			type_id<U>()
 		)
 	{
 	}

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<decltype(tid.id())>{}(tid.id());
+			}
+			else {
+				return static_cast<std::size_t>(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;

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<std::nullptr_t>())
 		{
 			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<LocalData>(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<std::nullptr_t>();
+		m_local->user_data_type = type_id<std::nullptr_t>();
 	}
 
 	bool VM::has_user_data() const noexcept {

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);
 		}