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
