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