xo-reflect: refactor to support manually-constructed function types

include/xo/reflect/EstablishTypeDescr.hpp

@@ -40,7 +40,7 @@ namespace xo {
             template<typename T>
             static TypeDescrW establish() {
                 TypeDescrW td = TypeDescrBase::require(&typeid(T),
-                                                       type_name<T>(),
+                                                       std::string(type_name<T>()),
                                                        nullptr);
 
 #ifdef NOT_USING

include/xo/reflect/TypeDescr.hpp

@@ -116,6 +116,50 @@ namespace xo {
         } /*namespace detail*/
 #endif
 
+        /* hashable contents of a FunctionTdx instance (without requiring decl of TypeDescrExtra),
+         * for unique-ification of manually-constructed function types
+         */
+        struct FunctionTdxInfo {
+            FunctionTdxInfo() = default;
+            FunctionTdxInfo(TypeDescr retval_td,
+                            const std::vector<TypeDescr> & arg_td_v,
+                            bool is_noexcept)
+                : retval_td_{retval_td},
+                  arg_td_v_{arg_td_v},
+                  is_noexcept_{is_noexcept}
+                {}
+
+            /** compare two FunctionTdxInfo objects for equality
+             **/
+            inline bool operator==(const FunctionTdxInfo & other) const noexcept {
+                if (retval_td_ != other.retval_td_)
+                    return true;
+                if (arg_td_v_.size() != other.arg_td_v_.size())
+                    return false;
+
+                for (std::size_t i = 0, n = arg_td_v_.size(); i < n; ++i) {
+                    if (arg_td_v_[i] != other.arg_td_v_[i])
+                        return false;
+                }
+
+                if (is_noexcept_ != other.is_noexcept_)
+                    return false;
+
+                return true;
+            }
+
+            /** function return value **/
+            TypeDescr retval_td_ = nullptr;
+            /** function arguments,  in positional order **/
+            std::vector<TypeDescr> arg_td_v_;
+            /** true iff function promises never to throw **/
+            bool is_noexcept_ = false;
+        };
+    }
+}
+
+namespace xo {
+    namespace reflect {
         class TypeDescrExtra;
 
         /* run-time description for a native c++ type */
@@ -130,8 +174,8 @@ namespace xo {
              * introducing this for unit testing
              */
             static bool is_reflected(std::type_info const * tinfo) {
-                return (s_type_table_map.find(TypeInfoRef(tinfo))
+                return (s_native_type_table_map.find(TypeInfoRef(tinfo))
-                        != s_type_table_map.end());
+                        != s_native_type_table_map.end());
             } /*is_reflected*/
 
             /* NOTE:
@@ -141,17 +185,17 @@ namespace xo {
              *   See FAQ
              *     [Build Issues|Q2 - dynamic_cast<Foo<*>> fails]
              */
-            static TypeDescrW require(std::type_info const * tinfo,
+            static TypeDescrW require(const std::type_info * tinfo,
-                                      std::string_view canonical_name,
+                                      const std::string & canonical_name,
                                       std::unique_ptr<TypeDescrExtra> tdextra);
 
             /* print table of reflected types to os */
             static void print_reflected_types(std::ostream & os);
 
             TypeId id() const { return id_; }
-            std::type_info const * native_typeinfo() const { return native_typeinfo_; }
+            const std::type_info * native_typeinfo() const { return native_typeinfo_; }
-            std::string_view const & canonical_name() const { return canonical_name_; }
+            const std::string & canonical_name() const { return canonical_name_; }
-            std::string_view const & short_name() const { return short_name_; }
+            const std::string_view & short_name() const { return short_name_; }
             bool complete_flag() const { return complete_flag_; }
             TypeDescrExtra * tdextra() const { return tdextra_.get(); }
             Metatype metatype() const { return tdextra_->metatype(); }
@@ -273,6 +317,8 @@ namespace xo {
                 return *sm;
             } /*struct_member*/
 
+            /** nullptr for non-function types **/
+            const FunctionTdxInfo * fn_info() const { return this->tdextra_->fn_info(); }
             uint32_t n_fn_arg() const { return this->tdextra_->n_fn_arg(); }
 
             /* require:
@@ -298,29 +344,62 @@ namespace xo {
 
         private:
             TypeDescrBase(TypeId id,
-                          std::type_info const * tinfo,
+                          const std::type_info * tinfo,
-                          std::string_view canonical_name,
+                          const std::string & canonical_name,
                           std::unique_ptr<TypeDescrExtra> tdextra);
 
+            void assign_native_tinfo(const std::type_info * tinfo) {
+                assert(!native_typeinfo_);
+                native_typeinfo_ = tinfo;
+            }
+
         private:
             /* invariant:
              * - for all TypeDescrImpl instances x:
              *   - s_type_table_v[x->id()] = x
-             *   - s_type_table_map[TypeInfoRef(x->typeinfo())] = x
+             *   - s_native_type_table_map[TypeInfoRef(x->typeinfo())] = x
              */
 
-            /* hashmap of all native TypeDescr instances, indexed by typeinfo.
+            /** vector of all TypeDescr instances, indexed by TypeId.  singleton. **/
-             * singleton.
+            static std::vector<std::unique_ptr<TypeDescrBase>> s_type_table_v;
-             */
+
-            static std::unordered_map<TypeInfoRef, std::unique_ptr<TypeDescrBase>> s_type_table_map;
+            /** hashmap of all TypeDescr instances,
-            /* hashmap of (presumed) duplicate TypeInfoRef values.
+             *  indexed by canonical_name.
-             * This happens with clang sometimes when the same type is referenced
+             *
-             * from multiple modules (i.e. shared libs).
+             *  For manually-constructed TypeDescr instances
-             */
+             *  (see xo-expression for use-case) we require:
+             *
+             *  - TypeDescr::canonical_name uniquely identifies type
+             *  - to interact with an actually-equivalent type T
+             *    constructed by c++ compiler,  we need
+             *    to use the same canonical name that the compiler uses.
+             *
+             *    See type xo::reflect::type_name<>() [in demangle.hpp under xo-refcnt]
+             *    for implementation
+             **/
+            static std::unordered_map<std::string, TypeDescrBase*> s_canonical_type_table_map;
+
+            /** hashmap of all native TypeDescr instances,
+             *  indexed by typeinfo. singleton.
+             **/
+            static std::unordered_map<TypeInfoRef, TypeDescrBase *> s_native_type_table_map;
+
+            /** hashmap of (presumed) duplicate TypeInfoRef values.
+             *  This happens with clang sometimes when the same type is referenced
+             *  from multiple modules (i.e. shared libs).
+             **/
             static std::unordered_map<TypeInfoRef, TypeDescrBase *> s_coalesced_type_table_map;
 
-            /* vector of all TypeDescr instances.  singleton. */
+            /** map from a vector of TypeDescr objects:
-            static std::vector<TypeDescrBase *> s_type_table_v;
+             *    [Retval, Arg1, ...Argn]
+             *  to TypeDescr for function type
+             *    Retval(*)(Arg1..Argn)
+             *
+             *  Use these to unique-ify function types across:
+             *  - types sourced natively from c++ compiler
+             *  - types manually constructed (e.g. see Lambda.cpp in xo-expression)
+             **/
+            static std::unordered_map<FunctionTdxInfo, TypeDescrBase *> s_function_type_map;
 
         private:
             /* unique id# for this type */
@@ -332,15 +411,20 @@ namespace xo {
              *     see Lambda.cpp in xo-expression.
              **/
             std::type_info const * native_typeinfo_ = nullptr;
-            /* canonical name for this type (see demangle.hpp for type_name<T>())
+            /** canonical name for this type (see demangle.hpp for type_name<T>())
              * e.g.
              *   xo::option::Px2
-             */
+             *
-            std::string_view canonical_name_;
+             * NOTE: if we only had to deal with types created via Reflect::reflect<T>(),
-            /* suffix of .canonical_name, just after last ':'
+             *       then canonical_name could be string_view.  For manually-constructed
-             * e.g.
+             *       types,  there is no compiler-generated C-string constant to reference,
-             *   Px2
+             *       so need to use std::string here
-             */
+             **/
+            std::string canonical_name_;
+            /** substring .canonical_name, just after last ':'
+             *  e.g.
+             *    Px2
+             **/
             std::string_view short_name_;
             /* set to true once final value for .tdextra is established
              * intially all TypeDescr objects will use AtomicTdx for .tdextra
@@ -380,4 +464,24 @@ namespace xo {
     } /*namespace reflect*/
 } /*namespace xo*/
 
+namespace std {
+    /** @brief overload for hashing xo::reflect::FunctionTdxInfo objects
+     **/
+    template <>
+    struct hash<xo::reflect::FunctionTdxInfo> {
+        inline size_t operator()(const xo::reflect::FunctionTdxInfo & x) const noexcept {
+            /* we can hash on addresses,  since TypeDescr objects are immutable */
+            std::size_t h = hash<xo::reflect::TypeDescr>{}(x.retval_td_);
+
+            for (std::size_t i = 0, n = x.arg_td_v_.size(); i < n; ++i) {
+                h = (h << 1) ^ hash<xo::reflect::TypeDescr>{}(x.arg_td_v_[i]);
+            }
+
+            h = (h << 1) ^ (x.is_noexcept_ ? 1 : 0);
+
+            return h;
+        }
+    };
+}
+
 /* end TypeDescr.hpp */

include/xo/reflect/TypeDescrExtra.hpp

@@ -11,6 +11,7 @@ namespace xo {
     namespace reflect {
         /* forward-declaring here.  see [reflect/struct/StructMember.hpp] */
         class StructMember;
+        class FunctionTdxInfo;
         class TypeDescrBase;
         class TaggedPtr;
 
@@ -67,6 +68,7 @@ namespace xo {
              *
              *  @pre @ref TypeDescrExtra::is_function() is true
              **/
+            virtual const FunctionTdxInfo * fn_info() const { return nullptr; }
             virtual const TypeDescrBase * fn_retval() const { return nullptr; }
             virtual uint32_t n_fn_arg() const { return 0; }
             virtual const TypeDescrBase * fn_arg(uint32_t /*i_arg*/) const { return nullptr; }

include/xo/reflect/function/FunctionTdx.hpp

@@ -32,10 +32,11 @@ namespace xo {
             virtual TaggedPtr child_tp(uint32_t i, void * object) const override;
             const std::string & struct_member_name(uint32_t i) const override;
 
-            virtual TypeDescr fn_retval() const override { return retval_td_; }
+            virtual const FunctionTdxInfo * fn_info() const override { return &info_; }
-            virtual uint32_t n_fn_arg() const override { return arg_td_v_.size(); }
+            virtual TypeDescr fn_retval() const override { return info_.retval_td_; }
-            virtual TypeDescr fn_arg(uint32_t i) const override { return arg_td_v_[i]; }
+            virtual uint32_t n_fn_arg() const override { return info_.arg_td_v_.size(); }
-            virtual bool fn_is_noexcept() const override { return is_noexcept_; }
+            virtual TypeDescr fn_arg(uint32_t i) const override { return info_.arg_td_v_[i]; }
+            virtual bool fn_is_noexcept() const override { return info_.is_noexcept_; }
 
         private:
             FunctionTdx(TypeDescr retval_td,
@@ -43,12 +44,8 @@ namespace xo {
                         std::vector<TypeDescr> arg_td_v);
 
         private:
-            /** function return value **/
+            /** ingredients in complete function type description **/
-            TypeDescr retval_td_;
+            FunctionTdxInfo info_;
-            /** function arguments,  in positional order **/
-            std::vector<TypeDescr> arg_td_v_;
-            /** true iff function promises never to throw **/
-            bool is_noexcept_ = false;
         }; /*FunctionTdx*/
     } /*namespace reflect*/
 } /*namespace xo*/

src/reflect/TypeDescr.cpp

@@ -15,69 +15,135 @@ namespace xo {
         uint32_t
         TypeId::s_next_id = 1;
 
-        std::unordered_map<TypeInfoRef, std::unique_ptr<TypeDescrBase>>
+        std::unordered_map<FunctionTdxInfo, TypeDescrBase*>
-        TypeDescrBase::s_type_table_map;
+        TypeDescrBase::s_function_type_map;
+
+        std::unordered_map<std::string, TypeDescrBase*>
+        TypeDescrBase::s_canonical_type_table_map;
+
+        std::unordered_map<TypeInfoRef, TypeDescrBase*>
+        TypeDescrBase::s_native_type_table_map;
 
         std::unordered_map<TypeInfoRef, TypeDescrBase*>
         TypeDescrBase::s_coalesced_type_table_map;
 
-        std::vector<TypeDescrW>
+        std::vector<std::unique_ptr<TypeDescrBase>>
         TypeDescrBase::s_type_table_v;
 
         TypeDescrW
-        TypeDescrBase::require(std::type_info const * tinfo,
+        TypeDescrBase::require(const std::type_info * native_tinfo,
-                               std::string_view canonical_name,
+                               const std::string & canonical_name,
                                std::unique_ptr<TypeDescrExtra> tdextra)
         {
-            /* 1. lookup by tinfo hash_code in s_type_table_map */
+            if (native_tinfo) {
-            {
+                /* 1. lookup by tinfo hash_code in s_type_table_map
-                auto ix = s_type_table_map.find(TypeInfoRef(tinfo));
+                 *    Not available for manually-constructed type descriptions.
+                 */
+                {
+                    auto ix = s_native_type_table_map.find(TypeInfoRef(native_tinfo));
 
-                if ((ix != s_type_table_map.end()) && ix->second)
+                    if ((ix != s_native_type_table_map.end()) && ix->second)
-                    return ix->second.get();
+                        return ix->second;
-            }
+                }
 
-            /* 2. lookup by tinfo hash_code in s_coalesced_type_table_map */
+                /* 2. lookup by tinfo hash_code in s_coalesced_type_table_map */
-            {
+                {
-                auto ix = s_coalesced_type_table_map.find(TypeInfoRef(tinfo));
+                    auto ix = s_coalesced_type_table_map.find(TypeInfoRef(native_tinfo));
 
-                if ((ix != s_coalesced_type_table_map.end()) && ix->second)
+                    if ((ix != s_coalesced_type_table_map.end()) && ix->second)
-                    return ix->second;
+                        return ix->second;
-            }
-
-            /* 3. O(n) lookup by canonical_name,  before we create a new slot.
-             *
-             * Have to accept that on clang type_info objects aren't always unique (!$@#!!)
-             *
-             * TODO: lookup table keyed by canonical_name
-             */
-            for (TypeDescrBase * x : s_type_table_v) {
-                if (x && (x->canonical_name() == canonical_name)) {
-                    /* 1. assume *x represents the type associated with tinfo.
-                     * 2. *do* store tinfo in s_coalesced_type_table_map[],
-                     *    for faster lookup next time
-                     */
-                    s_coalesced_type_table_map[TypeInfoRef(tinfo)] = x;
-
-                    return x;
                 }
             }
 
-            TypeId id = TypeId::allocate();
+            /* 3. lookup by canonical_name,  before we create a new slot.
+             *
+             * Have to accept that on clang type_info objects aren't always unique (!$@#!!)
+             */
+            {
+                auto ix = s_canonical_type_table_map.find(canonical_name);
 
-            std::unique_ptr<TypeDescrBase> & slot = s_type_table_map[TypeInfoRef(tinfo)];
+                if (ix != s_canonical_type_table_map.end()) {
+                    /** assume existing slot, with same canonical name,
+                     *  represents the same type as native_tinfo
+                     **/
+                    if (native_tinfo) {
+                        auto existing_tinfo = ix->second->native_typeinfo();
 
-            slot.reset(new TypeDescrBase(id,
+                        /* given we have a match:
-                                         tinfo,
+                         * - on existing TypeDescr
-                                         canonical_name,
+                         * - with same canonical name as type assoc'd with native_tinfo
-                                         std::move(tdextra)));
+                         * then:
+                         *   it's possible existing TypeDescr was manually constructed
+                         *   (i.e. without capturing std::type_info).
+                         *
+                         * With that in mind, attach that typeinfo now
+                         */
+                        if (!existing_tinfo) {
+                            ix->second->assign_native_tinfo(native_tinfo);
 
-            if (s_type_table_v.size() <= id.id())
+                            s_native_type_table_map[TypeInfoRef(native_tinfo)]
-                s_type_table_v.resize(id.id() + 1);
+                                = ix->second;
+                        }
 
-            s_type_table_v[id.id()] = slot.get();
+                        if (existing_tinfo
+                            && (existing_tinfo != native_tinfo))
+                        {
+                            /* we have encountered distinct std::type_info objects
+                             * that appear to represent the same type.
+                             * (at least types with the same canonical name)
+                             *
+                             * We observe this happening sometimes with clang-prepared
+                             * shared libraries;  perhaps something going wrong with
+                             * symbol coalescing.
+                             *
+                             * Store the dups in s_coalesced_type_table_map for future reference.
+                             */
+                            auto jx = s_coalesced_type_table_map.find(TypeInfoRef(native_tinfo));
 
-            return slot.get();
+                            if (jx == s_coalesced_type_table_map.end())
+                                s_coalesced_type_table_map[TypeInfoRef(native_tinfo)]
+                                    = ix->second;
+                        }
+                    }
+
+                    return ix->second;
+                }
+            }
+
+            /* when control here:
+             * need type added to:
+             * - s_type_table_v
+             * - s_canonical_type_table_map
+             * - s_native_type_table_map
+             * - s_coalesced_type_table_map (omit, only used for dups)
+             * - s_function_type_map (if type represents a function)
+             */
+
+            /* allocate slot for a new TypeDescr instance: */
+
+            TypeId new_td_id = TypeId::allocate();
+
+            if (s_type_table_v.size() <= new_td_id.id())
+                s_type_table_v.resize(new_td_id.id() + 1);
+
+            auto & new_slot = s_type_table_v[new_td_id.id()];
+
+            auto new_td = new TypeDescrBase(new_td_id,
+                                            native_tinfo,
+                                            canonical_name,
+                                            std::move(tdextra));
+
+            new_slot.reset(new_td);
+
+            s_canonical_type_table_map[std::string(new_slot->canonical_name())] = new_td;
+            if (native_tinfo)
+                s_native_type_table_map[TypeInfoRef(native_tinfo)] = new_td;
+
+            if (new_td->tdextra() && new_td->is_function()) {
+                s_function_type_map[*(new_td->fn_info())] = new_td;
+            }
+
+            return new_slot.get();
         } /*require*/
 
         void
@@ -85,7 +151,7 @@ namespace xo {
         {
             os << "<type_table_v[" << s_type_table_v.size() << "]:";
 
-            for (TypeDescrBase * td : s_type_table_v) {
+            for (const auto & td : s_type_table_v) {
                 os << "\n ";
                 if (td) {
                     td->display(os);
@@ -124,8 +190,8 @@ namespace xo {
         } /*namespace*/
 
         TypeDescrBase::TypeDescrBase(TypeId id,
-                                     std::type_info const * native_tinfo,
+                                     const std::type_info * native_tinfo,
-                                     std::string_view canonical_name,
+                                     const std::string & canonical_name,
                                      std::unique_ptr<TypeDescrExtra> tdextra)
             : id_{std::move(id)},
               native_typeinfo_{native_tinfo},

src/reflect/function/FunctionTdx.cpp

@@ -19,9 +19,7 @@ namespace xo {
         FunctionTdx::FunctionTdx(TypeDescr retval_td,
                                  bool is_noexcept,
                                  std::vector<TypeDescr> arg_td_v)
-            : retval_td_{retval_td},
+            : info_{retval_td, std::move(arg_td_v), is_noexcept}
-              arg_td_v_{std::move(arg_td_v)},
-              is_noexcept_{is_noexcept}
         {
             if (!retval_td) {
                 throw std::runtime_error("FunctionTdx::ctor: null return type?");