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
===
--- sr
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
`Wind
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
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 dependenc
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
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-b
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_in
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.c
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:
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-co
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
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 d
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
compnerd marked an inline comment as done.
compnerd added inline comments.
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 si
compnerd updated this revision to Diff 82815.
compnerd added a comment.
Add suffixes
Repository:
rL LLVM
https://reviews.llvm.org/D28212
Files:
include/typeinfo
src/typeinfo.cpp
Index: src/typeinfo.cpp
===
--- src/typeinfo.
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 def
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
__
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
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
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?
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 o
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 structur
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
===
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/D2821
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-comm
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/ty
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 obj
27 matches
Mail list logo
