[PATCH] D28212: typeinfo: provide a partial implementation for Win32

[Eric Fiselier via Phabricator via cfe-commits]

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