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

Reply via email to