[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd closed this revision. compnerd added a comment. SVN r313344 https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd marked 4 inline comments as done. compnerd added a comment. Also added `_NOEXCEPT` to `__compare`. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
smeenai added inline comments. Comment at: include/typeinfo:100 +int __compare(const struct type_info &__rhs); +#endif // _LIBCPP_ABI_MICROSOFT + Drop the `struct`; it causes compilation errors. This needs to be marked `const`. Comment at: src/typeinfo.cpp:15 + if (&__data == &__rhs.__data) +return 0; + return strcmp(&__data.__decorated_name[1], &__rhs.__data.__decorated_name[1]); Drop the `struct` and add the `const`. Comment at: src/typeinfo.cpp:21 + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data.__decorated_name[1]; +} You need a `const` here. Comment at: src/typeinfo.cpp:36 +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } The `->` should be `.` https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd updated this revision to Diff 115326. https://reviews.llvm.org/D28212 Files: include/typeinfo src/support/runtime/exception_msvc.ipp src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -9,11 +9,48 @@ #include "typeinfo" +#if defined(_LIBCPP_ABI_MICROSOFT) +#include + +int std::type_info::__compare(const struct type_info &__rhs) { + if (&__data == &__rhs.__data) +return 0; + return strcmp(&__data.__decorated_name[1], &__rhs.__data.__decorated_name[1]); +} + +const char *std::type_info::name() _NOEXCEPT { + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data.__decorated_name[1]; +} + +size_t std::type_info::hash_code() const _NOEXCEPT { +#if defined(_WIN64) + constexpr size_t fnv_offset_basis = 14695981039346656037ull; + constexpr size_t fnv_prime = 10995116282110ull; +#else + constexpr size_t fnv_offset_basis = 2166136261ull; + constexpr size_t fnv_prime = 16777619ull; +#endif + + size_t value = fnv_offset_basis; + for (const char* c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} +#endif // _LIBCPP_ABI_MICROSOFT + // FIXME: Remove __APPLE__ default here once buildit is gone. -#if (!defined(_LIBCPP_ABI_MICROSOFT) && !defined(LIBCXX_BUILDING_LIBCXXABI) && \ -!defined(LIBCXXRT) && !defined(__GLIBCXX__) && \ -!defined(__APPLE__)) || \ -defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) // FIXME: remove this configuration. +// FIXME: Remove the _LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY configuration. +#if (!defined(LIBCXX_BUILDING_LIBCXXABI) && !defined(LIBCXXRT) && \ + !defined(__GLIBCXX__) && !defined(__APPLE__)) || \ +defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) std::type_info::~type_info() { } Index: src/support/runtime/exception_msvc.ipp === --- src/support/runtime/exception_msvc.ipp +++ src/support/runtime/exception_msvc.ipp @@ -86,4 +86,32 @@ return "bad_array_length"; } +bad_cast::bad_cast() _NOEXCEPT +{ +} + +bad_cast::~bad_cast() _NOEXCEPT +{ +} + +const char * +bad_cast::what() const _NOEXCEPT +{ + return "std::bad_cast"; +} + +bad_typeid::bad_typeid() _NOEXCEPT +{ +} + +bad_typeid::~bad_typeid() _NOEXCEPT +{ +} + +const char * +bad_typeid::what() const _NOEXCEPT +{ + return "std::bad_typeid"; +} + } // namespace std Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -69,18 +69,17 @@ #pragma GCC system_header #endif -#if defined(_LIBCPP_ABI_MICROSOFT) -#include -#elif defined(_LIBCPP_NONUNIQUE_RTTI_BIT) +#if !defined(_LIBCPP_ABI_MICROSOFT) +#if defined(_LIBCPP_NONUNIQUE_RTTI_BIT) #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO #else #define _LIBCPP_HAS_UNIQUE_TYPEINFO #endif +#endif namespace std // purposefully not using versioning namespace { -#if !defined(_LIBCPP_ABI_MICROSOFT) class _LIBCPP_EXCEPTION_ABI type_info { type_info& operator=(const type_info&); @@ -92,7 +91,17 @@ { return __builtin_strcmp(name(), __arg.name()); } #endif +#if defined(_LIBCPP_ABI_MICROSOFT) +mutable struct { + const char *__undecorated_name; + const char __decorated_name[1]; +} __data; + +int __compare(const struct type_info &__rhs); +#endif // _LIBCPP_ABI_MICROSOFT + protected: +#if !defined(_LIBCPP_ABI_MICROSOFT) #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) // A const char* with the non-unique RTTI bit possibly set. uintptr_t __type_name; @@ -106,11 +115,27 @@ _LIBCPP_INLINE_VISIBILITY explicit type_info(const char* __n) : __type_name(__n) {} #endif +#endif // ! _LIBCPP_ABI_MICROSOFT public: _LIBCPP_AVAILABILITY_TYPEINFO_VTABLE virtual ~type_info(); +#if defined(_LIBCPP_ABI_MICROSOFT) +const char *name() const _NOEXCEPT; + +_LIBCPP_INLINE_VISIBILITY +bool before(const type_info& __arg) const _NOEXCEPT { + return __compare(__arg) < 0; +} + +size_t hash_code() const _NOEXCEPT; + +_LIBCPP_INLINE_VISIBILITY +bool operator==(const type_info& __arg) const _NOEXCEPT { + return __compare(__arg) == 0; +} +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) _LIBCPP_INLINE_VISIBILITY const char* name() const _NOEXCEPT @@ -167,6 +192,7 @@ bool operator==(const type_info& __arg) const _NOEXCEPT { return __type_name == __arg.__type_name; } #endif +#endif // _LIBCPP_ABI_MICROSOFT _LIBCPP_INLINE_VISIBILITY bool operator!=(const type_info& __arg) const _NOEXCEPT @@ -191,8 +217,6 @@ virtual const char* what() const _NOEXCEPT; }; -#endif // !_LIBCPP_ABI_MICROSOFT - } // std _LIBCPP_BEGIN_NAMESPACE_STD ___ cfe-commits mailing
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd commandeered this revision. compnerd edited reviewers, added: smeenai; removed: compnerd. compnerd added inline comments. Comment at: include/typeinfo:100 + + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); EricWF wrote: > These should probably have `_LIBCPP_FUNC_VIS` visibility declarations > attached to them. Wont bother due to out-of-line definition Comment at: include/typeinfo:101 + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); + static int __compare(const struct type_info::__data* __lls, EricWF wrote: > Is there a reason why `__name` and `__hash` need wrappers? Can't we just > provide out-of-line definitions for `name()` and `hash_code()` directly? I don't remember why ... we can address that when we hit it. Going to out-of-line them. Comment at: include/typeinfo:200 #endif +#endif EricWF wrote: > Comment on `#endif` please. Sure! Comment at: include/typeinfo:109 protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else smeenai wrote: > compnerd wrote: > > EricWF wrote: > > > Don't we need a constructor here? > > No, we do not need a constructor since the emitted object is already a well > > formed object instance. > I believe @EricWF was referring to the internal constructors that take a > `const char *`. Do we need one of those for the Microsoft typeinfo? No, no such constructor is needed with MS ABI AFAIK. Construction of the type does not invoke a constructor and libc++ does not create instances of this. Comment at: src/typeinfo.cpp:28-32 + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; EricWF wrote: > compnerd wrote: > > rnk wrote: > > > compnerd wrote: > > > > majnemer wrote: > > > > > majnemer wrote: > > > > > > Why make these static? Seems strange to use that storage duration. > > > > > These literals are ill-formed, I think you need a ULL suffix here. > > > > Oh, just to ensure that they are handled as literal constants. I can > > > > drop the static if you like, since the compiler should do the right > > > > thing anyways. > > > Isn't `constexpr const` redundant? > > Yeah, intentional. I should be using `_LIBCPP_CONSTEXPR` just incase the > > compiler doesn't support constexpr. > All supported compiler provide `constexpr` when building the dylib, so you > can assume we have it. > > I have no strong objection to the redundancy, but I'm not opposed to removing > either `const` or `constexpr`. Removed `static` and `const`. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. Woops. didn't mean to reject. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. - List Item In https://reviews.llvm.org/D28212#871034, @smeenai wrote: > @compnerd, @EricWF and I discussed this a bit on IRC yesterday. > > In this particular case, however, I don't believe those concerns apply. > `vcruntime_typeinfo.h` is only pulled in from MSVC's `typeinfo` header, so > it's not going to get pulled in unless explicitly requested, and therefore > there are no interoperability concerns. Additionally, the `type_info` > structure here is completely compatible with the one from the vcruntime > headers, since they both model the Microsoft ABI typeinfo structure. The only > behavior difference is that vcruntime's implementation will demangle the type > name, whereas this one won't, but we can address that in a follow-up. In > other words, I believe this change can go in independent of whatever decision > we reach for vcruntime interoperability in the general case. I'm convinced. Kill away. This patch LGTM other than the mentioned nits. Comment at: include/typeinfo:95 +#if defined(_LIBCPP_ABI_MICROSOFT) + mutable struct __data { +const char *__undecorated_name; Perhaps the struct should have a different name than the instance of it. Comment at: include/typeinfo:100 + + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); These should probably have `_LIBCPP_FUNC_VIS` visibility declarations attached to them. Comment at: include/typeinfo:101 + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); + static int __compare(const struct type_info::__data* __lls, Is there a reason why `__name` and `__hash` need wrappers? Can't we just provide out-of-line definitions for `name()` and `hash_code()` directly? Comment at: include/typeinfo:200 #endif +#endif Comment on `#endif` please. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
smeenai added a comment. @compnerd, @EricWF and I discussed this a bit on IRC yesterday. My motivation here is that I'm using libc++ with other headers that clash badly with the vcruntime headers, which prevents me from using libc++'s `_LIBCPP_ABI_MICROSOFT` support. Reducing libc++'s dependencies on the vcruntime headers enables me to at least use parts of the Microsoft ABI support. At the same time, @EricWF pointed out that most people will expect libc++ to play nicely with the vcruntime headers. In particular, `vcruntime_new.h` ends up getting pulled in from all sorts of places, which is why libc++ defers to that header instead of trying to provide its own new/delete declarations for `_LIBCPP_ABI_MICROSOFT`. One option would be an alternate ABI configuration, which is basically "Microsoft ABI support without reliance on and interoperability with vcruntime headers". Another possibility would be for the libc++ headers to provide vcruntime-compatible declarations for `_LIBCPP_ABI_MICROSOFT`, which would allow us to remain compatible with the vcruntime headers without depending on them. I find the second option to be cleaner conceptually and would prefer it. In this particular case, however, I don't believe those concerns apply. `vcruntime_typeinfo.h` is only pulled in from MSVC's `typeinfo` header, so it's not going to get pulled in unless explicitly requested, and therefore there are no interoperability concerns. Additionally, the `type_info` structure here is completely compatible with the one from the vcruntime headers, since they both model the Microsoft ABI typeinfo structure. The only behavior difference is that vcruntime's implementation will demangle the type name, whereas this one won't, but we can address that in a follow-up. In other words, I believe this change can go in independent of whatever decision we reach for vcruntime interoperability in the general case. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd added a comment. I'm happy to pick this up again. LMK what needs to be addressed. AFAIK, no constructors will be invoked as the class is just a wrapper over static data so we don't need the constructors. I would rather do the undecorating stuff in a follow-up. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
smeenai added a comment. Rebased and tested on Windows again. I'd like to see this patch get through to reduce the dependencies on the `vcruntime` headers, since those end up pulling in lots of cruft and can cause some nasty conflicts. (Similarly, I'll be trying to remove dependencies on `Windows.h`, which is an evil, evil header.) There's a behavior difference between this implementation and vcruntime's, since this one doesn't demangle the type name. Idk how much that difference matters in practice though, and I think it's fine to address in a follow-up. What else needs to be addressed before this is good to go? Comment at: include/typeinfo:109 protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else compnerd wrote: > EricWF wrote: > > Don't we need a constructor here? > No, we do not need a constructor since the emitted object is already a well > formed object instance. I believe @EricWF was referring to the internal constructors that take a `const char *`. Do we need one of those for the Microsoft typeinfo? https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
smeenai updated this revision to Diff 114957. smeenai added a comment. Rebase https://reviews.llvm.org/D28212 Files: include/typeinfo src/support/runtime/exception_msvc.ipp src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -9,11 +9,51 @@ #include "typeinfo" -// FIXME: Remove __APPLE__ default here once buildit is gone. -#if (!defined(_LIBCPP_ABI_MICROSOFT) && !defined(LIBCXX_BUILDING_LIBCXXABI) && \ -!defined(LIBCXXRT) && !defined(__GLIBCXX__) && \ -!defined(__APPLE__)) || \ -defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) // FIXME: remove this configuration. +#if defined(_LIBCPP_ABI_MICROSOFT) +#include + +const char* std::type_info::__name(const struct std::type_info::__data* __data) +{ + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data->__decorated_name[1]; +} + +size_t std::type_info::__hash(const struct std::type_info::__data* __data) +{ +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037ull; + static constexpr const size_t fnv_prime = 10995116282110ull; +#else + static constexpr const size_t fnv_offset_basis = 2166136261ull; + static constexpr const size_t fnv_prime = 16777619ull; +#endif + + size_t value = fnv_offset_basis; + for (const char *c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} + +int std::type_info::__compare(const struct std::type_info::__data* __lhs, + const struct std::type_info::__data* __rhs) +{ + if (__lhs == __rhs) +return 0; + return strcmp(&__lhs->__decorated_name[1], &__rhs->__decorated_name[1]); +} +#endif + +#if (!defined(LIBCXX_BUILDING_LIBCXXABI) && \ + !defined(LIBCXXRT) && !defined(__GLIBCXX__) && \ + !defined(__APPLE__)) || \ + defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) // FIXME: remove this configuration. std::type_info::~type_info() { } Index: src/support/runtime/exception_msvc.ipp === --- src/support/runtime/exception_msvc.ipp +++ src/support/runtime/exception_msvc.ipp @@ -86,4 +86,33 @@ return "bad_array_length"; } + +bad_cast::bad_cast() _NOEXCEPT +{ +} + +bad_typeid::bad_typeid() _NOEXCEPT +{ +} + +bad_cast::~bad_cast() _NOEXCEPT +{ +} + +const char* +bad_cast::what() const _NOEXCEPT +{ + return "std::bad_cast"; +} + +bad_typeid::~bad_typeid() _NOEXCEPT +{ +} + +const char* +bad_typeid::what() const _NOEXCEPT +{ + return "std::bad_typeid"; +} + } // namespace std Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -69,18 +69,17 @@ #pragma GCC system_header #endif -#if defined(_LIBCPP_ABI_MICROSOFT) -#include -#elif defined(_LIBCPP_NONUNIQUE_RTTI_BIT) +#if !defined(_LIBCPP_ABI_MICROSOFT) +#if defined(_LIBCPP_NONUNIQUE_RTTI_BIT) #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO #else #define _LIBCPP_HAS_UNIQUE_TYPEINFO #endif +#endif namespace std // purposefully not using versioning namespace { -#if !defined(_LIBCPP_ABI_MICROSOFT) class _LIBCPP_EXCEPTION_ABI type_info { type_info& operator=(const type_info&); @@ -92,7 +91,20 @@ { return __builtin_strcmp(name(), __arg.name()); } #endif +#if defined(_LIBCPP_ABI_MICROSOFT) + mutable struct __data { +const char *__undecorated_name; +const char __decorated_name[1]; + } __data; + + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); + static int __compare(const struct type_info::__data* __lls, + const struct type_info::__data* __rhs); +#endif + protected: +#if !defined(_LIBCPP_ABI_MICROSOFT) #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) // A const char* with the non-unique RTTI bit possibly set. uintptr_t __type_name; @@ -106,11 +118,29 @@ _LIBCPP_INLINE_VISIBILITY explicit type_info(const char* __n) : __type_name(__n) {} #endif +#endif public: _LIBCPP_AVAILABILITY_TYPEINFO_VTABLE virtual ~type_info(); +#if defined(_LIBCPP_ABI_MICROSOFT) +_LIBCPP_INLINE_VISIBILITY +const char *name() const _NOEXCEPT +{ return __name(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool before(const type_info& __arg) const _NOEXCEPT +{ return __compare(&__data, &__arg.__data) < 0; } + +_LIBCPP_INLINE_VISIBILITY +size_t hash_code() const _NOEXCEPT +{ return __hash(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool operator==(const type_info& __arg) const _NOEXCEPT +{ return __compare(&__data, &__arg.__data) == 0; } +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) _LIBCPP_INLINE_VISIBILITY const char* name() const _NOEXCEPT @@ -167,6 +197,7 @@ bool
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd added inline comments. Comment at: include/typeinfo:109 protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else EricWF wrote: > Don't we need a constructor here? No, we do not need a constructor since the emitted object is already a well formed object instance. Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd updated this revision to Diff 83489. compnerd added a comment. Rebase for r291329. Repository: rL LLVM https://reviews.llvm.org/D28212 Files: include/typeinfo src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -15,6 +15,47 @@ #include "typeinfo" +#if defined(_LIBCPP_ABI_MICROSOFT) +#include + +const char* std::type_info::__name(const struct std::type_info::__data* __data) +{ + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data->__decorated_name[1]; +} + +size_t std::type_info::__hash(const struct std::type_info::__data* __data) +{ +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037ull; + static constexpr const size_t fnv_prime = 10995116282110ull; +#else + static constexpr const size_t fnv_offset_basis = 2166136261ull; + static constexpr const size_t fnv_prime = 16777619ull; +#endif + + size_t value = fnv_offset_basis; + for (const char *c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} + +int std::type_info::__compare(const struct std::type_info::__data* __lhs, + const struct std::type_info::__data* __rhs) +{ + if (__lhs == __rhs) +return 0; + return strcmp(&__lhs->__decorated_name[1], &__rhs->__decorated_name[1]); +} +#endif + #if defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) std::type_info::~type_info() { Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -69,11 +69,13 @@ #pragma GCC system_header #endif +#if !defined(_LIBCPP_ABI_MICROSOFT) #if defined(_LIBCPP_NONUNIQUE_RTTI_BIT) #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO #else #define _LIBCPP_HAS_UNIQUE_TYPEINFO #endif +#endif namespace std // purposefully not using versioning namespace { @@ -89,7 +91,21 @@ { return __builtin_strcmp(name(), __arg.name()); } #endif +#if defined(_LIBCPP_ABI_MICROSOFT) + mutable struct __data { +const char *__undecorated_name; +const char __decorated_name[1]; + } __data; + + static const char *__name(const struct type_info::__data *__data); + static size_t __hash(const struct type_info::__data *__data); + static int __compare(const struct type_info::__data* __lls, + const struct type_info::__data* __rhs); +#endif + protected: +#if defined(_LIBCPP_ABI_MICROSOFT) +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) // A const char* with the non-unique RTTI bit possibly set. uintptr_t __type_name; @@ -102,10 +118,28 @@ _LIBCPP_INLINE_VISIBILITY type_info(const char* __n) : __type_name(__n) {} #endif +#endif public: virtual ~type_info(); +#if defined(_LIBCPP_ABI_MICROSOFT) +_LIBCPP_INLINE_VISIBILITY +const char *name() const _NOEXCEPT +{ return __name(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool before(const type_info& __arg) const _NOEXCEPT +{ return __compare(&__data, &__arg.__data) < 0; } + +_LIBCPP_INLINE_VISIBILITY +size_t hash_code() const _NOEXCEPT +{ return __hash(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool operator==(const type_info& __arg) const _NOEXCEPT +{ return __compare(&__data, &__arg.__data) == 0; } +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) _LIBCPP_INLINE_VISIBILITY const char* name() const _NOEXCEPT @@ -162,6 +196,7 @@ bool operator==(const type_info& __arg) const _NOEXCEPT { return __type_name == __arg.__type_name; } #endif +#endif _LIBCPP_INLINE_VISIBILITY bool operator!=(const type_info& __arg) const _NOEXCEPT ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF added a comment. Nevermind. Missed the dependency. Would you be willing to kill that dependency for now and we can fix it once we get Clang support? Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF added a comment. This isn't applying against master. Could you rebase? Comment at: include/typeinfo:109 protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else Don't we need a constructor here? Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd set the repository for this revision to rL LLVM. compnerd updated this revision to Diff 83326. Repository: rL LLVM https://reviews.llvm.org/D28212 Files: include/typeinfo src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -15,6 +15,45 @@ #include "typeinfo" +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#include + +const char *std::type_info::__name(std::type_info::__data *__data) { + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data->__decorated_name[1]; +} + +size_t std::type_info::__hash(const std::type_info::__data *__data) { +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037ull; + static constexpr const size_t fnv_prime = 10995116282110ull; +#else + static constexpr const size_t fnv_offset_basis = 2166136261ull; + static constexpr const size_t fnv_prime = 16777619ull; +#endif + + size_t value = fnv_offset_basis; + for (const char *c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} + +int std::type_info::__compare(const std::type_info::__data* __lhs, + const std::type_info::__data* __rhs) +{ + if (__lhs == __rhs) +return 0; + return strcmp(&__lhs->__decorated_name[1], &__rhs->__decorated_name[1]); +} +#endif + #if defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) std::type_info::~type_info() { Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -69,11 +69,15 @@ #pragma GCC system_header #endif +#if defined(_LIBCPP_ABI_MS) +#define _LIBCPP_HAS_MS_ABI_TYPEINFO +#else #if defined(_LIBCPP_NONUNIQUE_RTTI_BIT) #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO #else #define _LIBCPP_HAS_UNIQUE_TYPEINFO #endif +#endif namespace std // purposefully not using versioning namespace { @@ -89,7 +93,21 @@ { return __builtin_strcmp(name(), __arg.name()); } #endif +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) + mutable struct __data { +const char *__undecorated_name; +const char __decorated_name[1]; + } __data; + + static const char *__name(const type_info::__data *__data); + static size_t __hash(const type_info:__data *__data); + static int compare(const type_info::__data* __lhs, + const type_info::__data* __rhs); +#endif + protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) // A const char* with the non-unique RTTI bit possibly set. uintptr_t __type_name; @@ -102,10 +120,28 @@ _LIBCPP_INLINE_VISIBILITY type_info(const char* __n) : __type_name(__n) {} #endif +#endif public: virtual ~type_info(); +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +_LIBCPP_INLINE_VISIBILITY +const char *name() const _NOEXCEPT +{ return __name(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool before(const type_info& __arg) const _NOEXCEPT +{ return __compre(&__data, &__arg.__data) < 0; } + +_LIBCPP_INLINE_VISIBILITY +size_t hash_code() const _NOEXCEPT +{ return __hash(&__data); } + +_LIBCPP_INLINE_VISIBILITY +bool operator==(const type_info& __arg) const _NOEXCEPT +{ return compare(&__data, &__arg.__data) == 0; } +#else #if defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) _LIBCPP_INLINE_VISIBILITY const char* name() const _NOEXCEPT @@ -162,6 +198,7 @@ bool operator==(const type_info& __arg) const _NOEXCEPT { return __type_name == __arg.__type_name; } #endif +#endif _LIBCPP_INLINE_VISIBILITY bool operator!=(const type_info& __arg) const _NOEXCEPT ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF added inline comments. Comment at: include/typeinfo:75 +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else compnerd wrote: > rnk wrote: > > Is _WIN32 the right condition? It seems like this is intended to match the > > MS ABI RTTI structure, not the Itanium one. _MSC_VER maybe? > Yeah, this is meant to match the MS ABI RTTI. Yeah, _MSC_VER seems more > appropriate than _WIN32. I agree. Please make this change before committing. Comment at: include/typeinfo:193 + +type_info::type_info(const char* __n) +: __type_name(__n) {} This constructor is almost certainly not correct. @compnerd do you know what constructor call `clang-cl` generates to construct `type_info`? Comment at: src/typeinfo.cpp:28-32 + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; compnerd wrote: > rnk wrote: > > compnerd wrote: > > > majnemer wrote: > > > > majnemer wrote: > > > > > Why make these static? Seems strange to use that storage duration. > > > > These literals are ill-formed, I think you need a ULL suffix here. > > > Oh, just to ensure that they are handled as literal constants. I can > > > drop the static if you like, since the compiler should do the right thing > > > anyways. > > Isn't `constexpr const` redundant? > Yeah, intentional. I should be using `_LIBCPP_CONSTEXPR` just incase the > compiler doesn't support constexpr. All supported compiler provide `constexpr` when building the dylib, so you can assume we have it. I have no strong objection to the redundancy, but I'm not opposed to removing either `const` or `constexpr`. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd added inline comments. Comment at: include/typeinfo:75 +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else rnk wrote: > Is _WIN32 the right condition? It seems like this is intended to match the MS > ABI RTTI structure, not the Itanium one. _MSC_VER maybe? Yeah, this is meant to match the MS ABI RTTI. Yeah, _MSC_VER seems more appropriate than _WIN32. Comment at: src/typeinfo.cpp:28-32 + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; rnk wrote: > compnerd wrote: > > majnemer wrote: > > > majnemer wrote: > > > > Why make these static? Seems strange to use that storage duration. > > > These literals are ill-formed, I think you need a ULL suffix here. > > Oh, just to ensure that they are handled as literal constants. I can drop > > the static if you like, since the compiler should do the right thing > > anyways. > Isn't `constexpr const` redundant? Yeah, intentional. I should be using `_LIBCPP_CONSTEXPR` just incase the compiler doesn't support constexpr. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
rnk added inline comments. Comment at: include/typeinfo:75 +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else Is _WIN32 the right condition? It seems like this is intended to match the MS ABI RTTI structure, not the Itanium one. _MSC_VER maybe? Comment at: src/typeinfo.cpp:28-32 + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; compnerd wrote: > majnemer wrote: > > majnemer wrote: > > > Why make these static? Seems strange to use that storage duration. > > These literals are ill-formed, I think you need a ULL suffix here. > Oh, just to ensure that they are handled as literal constants. I can drop > the static if you like, since the compiler should do the right thing anyways. Isn't `constexpr const` redundant? https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd commandeered this revision. compnerd edited reviewers, added: EricWF; removed: compnerd. compnerd added a comment. Ill use this version for the commit, but Id like to get an okay on it before I do that :-). https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF removed rL LLVM as the repository for this revision. EricWF updated this revision to Diff 82825. EricWF added a comment. Update with the prefered style changes. @compnerd please still this revision back when ready. https://reviews.llvm.org/D28212 Files: include/typeinfo src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -14,13 +14,51 @@ #endif #include "typeinfo" +#include "cstring" #if defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY) std::type_info::~type_info() { } #endif +#if defined(_LIBCPP_HAS_WINDOWS_TYPEINFO) + +const char *std::type_info::__name(std::type_info::__data_t *__data) { + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data->__decorated_name[1]; +} + +size_t std::type_info::__hash(const std::type_info::__data_t *__data) { +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037ull; + static constexpr const size_t fnv_prime = 10995116282110ull; +#else + static constexpr const size_t fnv_offset_basis = 2166136261ull; + static constexpr const size_t fnv_prime = 16777619ull; +#endif + + size_t value = fnv_offset_basis; + for (const char *c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} + +int std::type_info::__compare(const std::type_info::__data_t *__lhs, + const std::type_info::__data_t *__rhs) { + if (__lhs == __rhs) +return 0; + return strcmp(&__lhs->__decorated_name[1], &__rhs->__decorated_name[1]); +} +#endif // _LIBCPP_HAS_WINDOWS_TYPEINFO + #if !defined(LIBCXXRT) && !defined(_LIBCPPABI_VERSION) std::bad_cast::bad_cast() _NOEXCEPT Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -69,86 +69,147 @@ #pragma GCC system_header #endif +#ifdef _LIBCPP_NONUNIQUE_RTTI_BIT +#define _LIBCPP_HAS_NONUNIQUE_TYPEINFO +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else +#define _LIBCPP_HAS_UNIQUE_TYPEINFO +#endif + namespace std // purposefully not using versioning namespace { class _LIBCPP_EXCEPTION_ABI type_info { type_info& operator=(const type_info&); type_info(const type_info&); +#if defined(_LIBCPP_HAS_WINDOWS_TYPEINFO) + static const char *__name(type_info::__data_t *__data); + static size_t __hash(const type_info::__data_t *__data); + static int __compare(const type_info::__data_t *__l, + const type_info::__data_t *__r); + + mutable struct __data_t { +const char *__undecorated_name; +const char __decorated_name[1]; + } __data; + +#elif defined(_LIBCPP_HAS_NONUNIQUE_TYPEINFO) + _LIBCPP_INLINE_VISIBILITY + int __compare_nonunique_names(const type_info &__arg) const _NOEXCEPT + { return __builtin_strcmp(name(), __arg.name()); } + protected: -#ifndef _LIBCPP_NONUNIQUE_RTTI_BIT -const char* __type_name; -#else -// A const char* with the non-unique RTTI bit possibly set. -uintptr_t __type_name; -#endif + uintptr_t __type_name // A const char* with the non-unique RTTI bit possibly set. -_LIBCPP_INLINE_VISIBILITY -explicit type_info(const char* __n) -#ifndef _LIBCPP_NONUNIQUE_RTTI_BIT -: __type_name(__n) {} #else -: __type_name(reinterpret_cast(__n)) {} +protected: + const char* __type_name; #endif +protected: + inline _LIBCPP_INLINE_VISIBILITY + explicit type_info(const char* __n); + public: -virtual ~type_info(); + virtual ~type_info(); -_LIBCPP_INLINE_VISIBILITY -const char* name() const _NOEXCEPT -#ifndef _LIBCPP_NONUNIQUE_RTTI_BIT -{return __type_name;} -#else -{return reinterpret_cast(__type_name & ~_LIBCPP_NONUNIQUE_RTTI_BIT);} -#endif + inline _LIBCPP_INLINE_VISIBILITY + const char* name() const _NOEXCEPT; -_LIBCPP_INLINE_VISIBILITY -bool before(const type_info& __arg) const _NOEXCEPT -#ifndef _LIBCPP_NONUNIQUE_RTTI_BIT -{return __type_name < __arg.__type_name;} -#else -{if (!((__type_name & __arg.__type_name) & _LIBCPP_NONUNIQUE_RTTI_BIT)) - return __type_name < __arg.__type_name; - return __compare_nonunique_names(__arg) < 0;} -#endif + inline _LIBCPP_INLINE_VISIBILITY + bool before(const type_info& __arg) const _NOEXCEPT; -_LIBCPP_INLINE_VISIBILITY -size_t hash_code() const _NOEXCEPT -#ifndef _LIBCPP_NONUNIQUE_RTTI_BIT -{return reinterpret_cast(__type_name);} -#else -{if (!(__type_name & _LIBCPP_NONUNIQUE_RTTI_BIT)) return __type_name; - const char *__ptr = name(); - size_t __hash = 5381; - while (unsigned char __c = static_cast(*__ptr++)) - __hash = (__hash * 33) ^ __c; - return __hash;} -#endif + inline _LIBCPP_INLINE_VISIBILITY + size_t hash_code()
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF commandeered this revision. EricWF edited reviewers, added: compnerd; removed: EricWF. EricWF added a comment. Stealing this revision to update with the requested style changes: @compnerd please steal this back. Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
EricWF added a comment. I would really rather see a single definition of the `typeinfo` class between all platforms, but I see how that could get messy. How about instead of declaring a new one we use the existing class definition, but move all of the method definitions out-of-line and then define them in their own `#ifdef` blocks? Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
majnemer added inline comments. Comment at: src/typeinfo.cpp:28-29 +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else majnemer wrote: > Why make these static? Seems strange to use that storage duration. These literals are ill-formed, I think you need a ULL suffix here. Comment at: src/typeinfo.cpp:28-32 + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; Why make these static? Seems strange to use that storage duration. Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28212: typeinfo: provide a partial implementation for Win32
compnerd created this revision. compnerd added reviewers: EricWF, mclow.lists, majnemer, rnk. compnerd added subscribers: smeenai, kastiglione, cfe-commits. compnerd set the repository for this revision to rL LLVM. The RTTI structure is different on Windows when building under MS ABI. Update the definition to reflect this. The structure itself contains an area for caching the undecorated name (which is 0-initialized). The decorated name has a bitfield followed by the linkage name. When std::type_info::name is invoked for the first time, the runtime should undecorate the name, cache it, and return the undecorated name. This requires access to an implementation of `__unDName`. For now, return the raw name. This uses the fnv-1a hash to hash the name of the RTTI. We could use an alternate hash (murmur? city?), but, this was the quickest to throw together. Repository: rL LLVM https://reviews.llvm.org/D28212 Files: include/typeinfo src/typeinfo.cpp Index: src/typeinfo.cpp === --- src/typeinfo.cpp +++ src/typeinfo.cpp @@ -15,6 +15,44 @@ #include "typeinfo" +#if defined(_WIN32) +#include + +const char *std::type_info::name(std::type_info::data *__data) { + // TODO(compnerd) cache demangled &__data.__decorated_name[1] + return &__data->__decorated_name[1]; +} + +size_t std::type_info::hash(const std::type_info::data *__data) { +#if defined(_WIN64) + static constexpr const size_t fnv_offset_basis = 14695981039346656037; + static constexpr const size_t fnv_prime = 10995116282110; +#else + static constexpr const size_t fnv_offset_basis = 2166136261; + static constexpr const size_t fnv_prime = 16777619; +#endif + + size_t value = fnv_offset_basis; + for (const char *c = &__data->__decorated_name[1]; *c; ++c) { +value ^= static_cast(static_cast(*c)); +value *= fnv_prime; + } + +#if defined(_WIN64) + value ^= value >> 32; +#endif + + return value; +} + +int std::type_info::compare(const std::type_info::data *__lhs, +const std::type_info::data *__rhs) { + if (__lhs == __rhs) +return 0; + return strcmp(&__lhs->__decorated_name[1], &__rhs->__decorated_name[1]); +} +#endif + #if !defined(LIBCXXRT) && !defined(_LIBCPPABI_VERSION) std::type_info::~type_info() Index: include/typeinfo === --- include/typeinfo +++ include/typeinfo @@ -72,6 +72,49 @@ namespace std // purposefully not using versioning namespace { +#if defined(_WIN32) +class _LIBCPP_EXCEPTION_ABI type_info +{ + type_info & operator=(const type_info &) _LIBCPP_EQUAL_DELETE; + type_info (const type_info &) _LIBCPP_EQUAL_DELETE; + + mutable struct data { +const char *__undecorated_name; +const char __decorated_name[1]; + } __data; + + static const char *name(type_info::data *__data); + static size_t hash(const type_info::data *__data); + static int compare(const type_info::data *__l, const type_info::data *__r); + +public: + virtual ~type_info() _NOEXCEPT; + + _LIBCPP_INLINE_VISIBILITY + const char *name() const _NOEXCEPT { +return name(&__data); + } + + _LIBCPP_INLINE_VISIBILITY + bool before(const type_info &__arg) const _NOEXCEPT { +return compare(&__data, &__arg.__data) < 0; + } + + _LIBCPP_INLINE_VISIBILITY + size_t hash_code() const _NOEXCEPT { +return hash(&__data); + } + + _LIBCPP_INLINE_VISIBILITY + bool operator==(const type_info &__arg) const _NOEXCEPT { +return compare(&__data, &__arg.__data) == 0; + } + + bool operator!=(const type_info &__arg) const _NOEXCEPT { +return compare(&__data, &__arg.__data) != 0; + } +}; +#else class _LIBCPP_EXCEPTION_ABI type_info { type_info& operator=(const type_info&); @@ -148,6 +191,7 @@ {return __builtin_strcmp(name(), __arg.name());} #endif }; +#endif class _LIBCPP_EXCEPTION_ABI bad_cast : public exception ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits