Remove type map (#161)

* Remove type map

* Use hashmap for type mapping only on Windows

See comments in jlcxx.cpp for details

Author: barche

include/jlcxx/jlcxx_config.hpp

@@ -2,6 +2,7 @@
 #define JLCXX_CONFIG_HPP
 
 #ifdef _WIN32
+  #define JLCXX_USE_TYPE_MAP
   #ifdef JLCXX_EXPORTS
       #define JLCXX_API __declspec(dllexport)
   #else
@@ -14,8 +15,8 @@
 #endif
 
 #define JLCXX_VERSION_MAJOR 0
-#define JLCXX_VERSION_MINOR 12
-#define JLCXX_VERSION_PATCH 5
+#define JLCXX_VERSION_MINOR 13
+#define JLCXX_VERSION_PATCH 0
 
 // From https://stackoverflow.com/questions/5459868/concatenate-int-to-string-using-c-preprocessor
 #define __JLCXX_STR_HELPER(x) #x

include/jlcxx/module.hpp

@@ -1235,6 +1235,10 @@ class TypeWrapper
 
 using TypeWrapper1 = TypeWrapper<Parametric<TypeVar<1>>>;
 
+#ifdef JLCXX_USE_TYPE_MAP
+JLCXX_API std::shared_ptr<TypeWrapper1>& jlcxx_smartpointer_type(std::type_index idx);
+#endif
+
 template<typename ApplyT, typename... TypeLists> using combine_types = typename CombineTypes<ApplyT, TypeLists...>::type;
 
 template<typename T>

include/jlcxx/smart_pointers.hpp

@@ -186,13 +186,34 @@ struct SmartPtrMethods<PtrT<PointeeT, ExtraArgs...>, OtherPtrT>
 
 }
 
-JLCXX_API void set_smartpointer_type(const type_hash_t& hash, TypeWrapper1* new_wrapper);
-JLCXX_API TypeWrapper1* get_smartpointer_type(const type_hash_t& hash);
+template<typename T>
+inline std::shared_ptr<TypeWrapper1>& stored_smartpointer_type()
+{
+#ifdef JLCXX_USE_TYPE_MAP
+  return jlcxx_smartpointer_type(typeid(T));
+#else
+  static std::shared_ptr<TypeWrapper1> m_ptr;
+  return m_ptr;
+#endif
+}
+
+template<typename T>
+void set_smartpointer_type(TypeWrapper1* new_wrapper)
+{
+  assert(stored_smartpointer_type<T>().get() == nullptr);
+  stored_smartpointer_type<T>().reset(new_wrapper);
+}
+
+template<typename T>
+TypeWrapper1* get_smartpointer_type()
+{
+  return stored_smartpointer_type<T>().get();
+}
 
 template<template<typename...> class T>
 TypeWrapper1 smart_ptr_wrapper(Module& module)
 {
-  static TypeWrapper1* stored_wrapper = get_smartpointer_type(type_hash<T<int>>());
+  static TypeWrapper1* stored_wrapper = get_smartpointer_type<T<int>>();
   if(stored_wrapper == nullptr)
   {
     std::cerr << "Smart pointer type has no wrapper" << std::endl;
@@ -220,7 +241,7 @@ template<template<typename...> class T>
 TypeWrapper1& add_smart_pointer(Module& mod, const std::string& name)
 {
   TypeWrapper1* tw = new TypeWrapper1(mod.add_type<Parametric<TypeVar<1>>>(name, julia_type("SmartPointer", get_cxxwrap_module())));
-  smartptr::set_smartpointer_type(type_hash<T<int>>(), tw);
+  smartptr::set_smartpointer_type<T<int>>(tw);
   return *tw;
 }
 
@@ -284,7 +305,7 @@ struct julia_type_factory<T, CxxWrappedTrait<SmartPointerTrait>>
     detail::apply_smart_ptr_type<ConstMappedT>()(curmod);
     smartptr::detail::SmartPtrMethods<NonConstMappedT, typename ConstructorPointerType<NonConstMappedT>::type>::apply(curmod);
     assert(has_julia_type<T>());
-    return JuliaTypeCache<T>::julia_type();
+    return stored_type<T>().get_dt();
   }
 };
 

include/jlcxx/stl.hpp

@@ -351,7 +351,7 @@ struct julia_type_factory<std::vector<T>>
     Module& curmod = registry().current_module();
     stl::apply_stl<T>(curmod);
     assert(has_julia_type<MappedT>());
-    return JuliaTypeCache<MappedT>::julia_type();
+    return stored_type<MappedT>().get_dt();
   }
 };
 

include/jlcxx/type_conversion.hpp

@@ -322,7 +322,7 @@ struct static_type_mapping<BoxedValue<T>>
 template<typename T> using static_julia_type = typename static_type_mapping<T>::type;
 
 // Store a data type pointer, ensuring GC safety
-struct CachedDatatype
+struct JLCXX_API CachedDatatype
 {
   explicit CachedDatatype() : m_dt(nullptr) {}
   explicit CachedDatatype(jl_datatype_t* dt, bool protect = true)
@@ -348,112 +348,64 @@ struct CachedDatatype
   jl_datatype_t* m_dt = nullptr;
 };
 
-// Work around the fact that references aren't part of the typeid result
-using type_hash_t = std::pair<std::type_index, std::size_t>;
 
-} // namespace jlcxx
+#ifdef JLCXX_USE_TYPE_MAP
 
-namespace std {
-
-// Hash implementation from https://en.cppreference.com/w/cpp/utility/hash
-template<>
-struct hash<jlcxx::type_hash_t> 
-{
-  std::size_t operator()(const jlcxx::type_hash_t& h) const noexcept
-  {
-    std::size_t h1 = std::hash<std::type_index>{}(h.first);
-    std::size_t h2 = std::hash<std::size_t>{}(h.second);
-    return h1 ^ (h2 << 1);
-  }
-};
-
-}
-
-namespace jlcxx
-{
-
-namespace detail
-{
+JLCXX_API CachedDatatype& jlcxx_type(std::type_index idx);
+JLCXX_API CachedDatatype& jlcxx_reftype(std::type_index idx);
+JLCXX_API CachedDatatype& jlcxx_constreftype(std::type_index idx);
 
 template<typename T>
-struct TypeHash
+struct HashedCache
 {
-  static inline type_hash_t value()
+  static inline CachedDatatype& value()
   {
-    return std::make_pair(std::type_index(typeid(T)), std::size_t(0));
+    return jlcxx_type(typeid(T));
   }
 };
 
 template<typename T>
-struct TypeHash<T&>
+struct HashedCache<T&>
 {
-  static inline type_hash_t value()
+  static inline CachedDatatype& value()
   {
-    return std::make_pair(std::type_index(typeid(T)), std::size_t(1));
+    return jlcxx_reftype(typeid(T));
   }
 };
 
 template<typename T>
-struct TypeHash<const T&>
+struct HashedCache<const T&>
 {
-  static inline type_hash_t value()
+  static inline CachedDatatype& value()
   {
-    return std::make_pair(std::type_index(typeid(T)), std::size_t(2));
+    return jlcxx_constreftype(typeid(T));
   }
 };
 
-}
+#endif
 
-template<typename T>
-inline type_hash_t type_hash()
+template<typename CppT>
+CachedDatatype& stored_type()
 {
-  return detail::TypeHash<T>::value();
+#ifdef JLCXX_USE_TYPE_MAP
+  return HashedCache<CppT>::value();
+#else
+  static CachedDatatype m_dt;
+  return m_dt;
+#endif
 }
 
-JLCXX_API std::unordered_map<type_hash_t, CachedDatatype>& jlcxx_type_map();
-
-/// Store the Julia datatype linked to SourceT
-template<typename SourceT>
-class JuliaTypeCache
-{
-public:
-
-  static inline jl_datatype_t* julia_type()
-  {
-    const auto result = jlcxx_type_map().find(type_hash<SourceT>());
-    if(result == jlcxx_type_map().end())
-    {
-      throw std::runtime_error("Type " + std::string(typeid(SourceT).name()) + " has no Julia wrapper");
-    }
-    return result->second.get_dt();
-  }
-
-  static inline void set_julia_type(jl_datatype_t* dt, bool protect = true)
-  {
-    type_hash_t new_hash = type_hash<SourceT>();
-    const auto [inserted_it, insert_success] = jlcxx_type_map().insert(std::make_pair(new_hash, CachedDatatype(dt, protect)));
-    if(!insert_success)
-    {
-      type_hash_t old_hash = inserted_it->first;
-      std::cout << "Warning: Type " << new_hash.first.name() << " already had a mapped type set as "
-        << julia_type_name(inserted_it->second.get_dt()) << " and const-ref indicator " << old_hash.second << " and C++ type name " << old_hash.first.name()
-        << ". Hash comparison: old(" << old_hash.first.hash_code() << "," << old_hash.second << ") == new(" << old_hash.first.hash_code() << "," << old_hash.second << ") == "
-        << std::boolalpha << (old_hash == new_hash) << std::endl;
-      return;
-    }
-  }
-
-  static inline bool has_julia_type()
-  {
-    const std::size_t nb_hits = jlcxx_type_map().count(type_hash<SourceT>());
-    return nb_hits != 0;
-  }
-};
-
 template<typename T>
 void set_julia_type(jl_datatype_t* dt, bool protect = true)
 {
-  JuliaTypeCache<typename std::remove_const<T>::type>::set_julia_type(dt, protect);
+  using nonconst_t = typename std::remove_const<T>::type;
+  CachedDatatype& cache = stored_type<nonconst_t>();
+  if(cache.get_dt() != nullptr)
+  {
+    std::cout << "Warning: Type " << typeid(T).name() << " already had a mapped type set as " << julia_type_name(cache.get_dt()) << std::endl;
+    return;
+  }
+  cache.set_dt(dt, protect);
 }
 
 /// Store the Julia datatype linked to SourceT
@@ -473,7 +425,11 @@ template<typename T>
 inline jl_datatype_t* julia_type()
 {
   using nonconst_t = typename std::remove_const<T>::type;
-  static jl_datatype_t* dt = JuliaTypeCache<nonconst_t>::julia_type();
+  jl_datatype_t* dt = stored_type<nonconst_t>().get_dt();
+  if(dt == nullptr)
+  {
+    throw std::runtime_error("Type " + std::string(typeid(nonconst_t).name()) + " has no Julia wrapper");
+  }
   return dt;
 }
 
@@ -482,7 +438,7 @@ template <typename T>
 bool has_julia_type()
 {
   using nonconst_t = typename std::remove_const<T>::type;
-  return JuliaTypeCache<nonconst_t>::has_julia_type();
+  return stored_type<nonconst_t>().get_dt() != nullptr;
 }
 
 /// Create the julia type associated with the given C++ type

src/jlcxx.cpp

@@ -341,38 +341,50 @@ namespace detail
   };
 }
 
-JLCXX_API std::unordered_map<type_hash_t, CachedDatatype>& jlcxx_type_map()
-{
-  static std::unordered_map<type_hash_t, CachedDatatype> m_map;
-  return m_map;
-}
+#ifdef JLCXX_USE_TYPE_MAP
 
-namespace smartptr
-{
+// On windows, we can't store a mapping from the C++ type to the Julia type
+// in a static variable declared in a template function, since each DLL
+// will have its onw copy of that variable, making it impossible to share
+// type definitions between the CxxWrap base library and other libraries.
+// The workaround is to store the types in a map, but this is more fragile
+// because the guarantees on std::type_index are not very strong either
+// in the context of sharing information between shared libraries. This is
+// why we fall back to this approach only on Windows. Using type names is
+// also not a solution because types in anonymous namespaces will clash.
+// Refs:
+// https://stackoverflow.com/questions/398069/static-member-variable-in-template-with-multiple-dlls
+// https://developercommunity.visualstudio.com/t/template-static-members-and-multiple-definitions-a/1202888
+// https://github.com/pybind/pybind11/pull/4319
+// https://bugs.llvm.org/show_bug.cgi?id=33542
+// https://github.com/pybind/pybind11/issues/3289
 
-std::map<type_hash_t, std::shared_ptr<TypeWrapper1>>& jlcxx_smartpointer_types()
+JLCXX_API CachedDatatype& jlcxx_type(std::type_index idx)
 {
-  static std::map<type_hash_t, std::shared_ptr<TypeWrapper1>> m_map;
-  return m_map;
+  static std::unordered_map<std::type_index, CachedDatatype> m_map;
+  return m_map.insert(std::make_pair(idx,CachedDatatype())).first->second;
 }
 
-JLCXX_API void set_smartpointer_type(const type_hash_t& hash, TypeWrapper1* new_wrapper)
+JLCXX_API CachedDatatype& jlcxx_reftype(std::type_index idx)
 {
-  jlcxx_smartpointer_types()[hash] = std::shared_ptr<TypeWrapper1>(new_wrapper);
+  static std::unordered_map<std::type_index, CachedDatatype> m_map;
+  return m_map.insert(std::make_pair(idx,CachedDatatype())).first->second;
 }
 
-JLCXX_API TypeWrapper1* get_smartpointer_type(const type_hash_t& hash)
+JLCXX_API CachedDatatype& jlcxx_constreftype(std::type_index idx)
 {
-  auto result = jlcxx_smartpointer_types().find(hash);
-  if(result == jlcxx_smartpointer_types().end())
-  {
-    return nullptr;
-  }
-  return result->second.get();
+  static std::unordered_map<std::type_index, CachedDatatype> m_map;
+  return m_map.insert(std::make_pair(idx,CachedDatatype())).first->second;
 }
 
+JLCXX_API std::shared_ptr<TypeWrapper1>& jlcxx_smartpointer_type(std::type_index idx)
+{
+  static std::unordered_map<std::type_index, std::shared_ptr<TypeWrapper1>> m_map;
+  return m_map.insert(std::make_pair(idx, nullptr)).first->second;
 }
 
+#endif
+
 JLCXX_API void register_core_types()
 {
   if(jl_base_module == nullptr)