EricWF added a comment.
In https://reviews.llvm.org/D28785#673227, @smeenai wrote:
> Still not a fan of the amount of vcruntime dependencies this is taking on,
> but I guess that can be followed up on.
Neither am I. However this gets the debug build working, and fixes RTTI, which
in turn
smeenai added a comment.
Still not a fan of the amount of vcruntime dependencies this is taking on, but
I guess that can be followed up on.
Was the `get_new_handler`/`set_new_handler` issue resolved? (The issue being
that those functions are in msvcprt, which our tests were silently linking
EricWF closed this revision.
EricWF added a comment.
Committed in r294707.
https://reviews.llvm.org/D28785
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
Accepting before committing.
I wasn't able to split out the MSVC only changes in a fruitful way, so I'm
going to commit this patch as-is.
https://reviews.llvm.org/D28785
EricWF updated this revision to Diff 87944.
EricWF added a comment.
Merge with upstream.
https://reviews.llvm.org/D28785
Files:
CMakeLists.txt
include/exception
include/new
include/typeinfo
src/exception.cpp
src/new.cpp
src/support/runtime/exception_fallback.ipp
EricWF added a comment.
In https://reviews.llvm.org/D28785#651156, @EricWF wrote:
> In https://reviews.llvm.org/D28785#650101, @compnerd wrote:
>
> > While I love this direction (the original version really was an
> > unintelligible pile of code), I really think that this change may be taking
EricWF added a comment.
In https://reviews.llvm.org/D28785#650101, @compnerd wrote:
> While I love this direction (the original version really was an
> unintelligible pile of code), I really think that this change may be taking
> on too much. Why not split it up first and do nothing else. We
compnerd added a comment.
While I love this direction (the original version really was an unintelligible
pile of code), I really think that this change may be taking on too much. Why
not split it up first and do nothing else. We could do the MS ABI
implementation in a subsequent change.
smeenai added a comment.
@EricWF and I discussed this on IRC a bunch yesterday.
We were inadvertently statically linking the Microsoft C++ libraries into
tests, which is why `std::set_new_handler` was working with this patch. With
that incorrect link removed, we get an undefined reference to
smeenai added inline comments.
Comment at: include/exception:85
+#if defined(_LIBCPP_ABI_MICROSOFT)
+#include
EricWF wrote:
> smeenai wrote:
> > What's the rationale for relying on Microsoft's exception implementation
> > rather than libc++'s?
>
EricWF updated this revision to Diff 84770.
EricWF added a comment.
Herald added a subscriber: mgorny.
- Allow standalone libc++ build on OS X to default to using `libc++abi`.
https://reviews.llvm.org/D28785
Files:
CMakeLists.txt
include/exception
include/new
include/typeinfo
EricWF added inline comments.
Comment at: include/new:177
+#if !defined(_LIBCPP_ABI_MICROSOFT)
+
EricWF wrote:
> smeenai wrote:
> > Might be helpful to have a comment explaining why we wanna defer these to
> > msvcrt on Windows?
> >
> > Also, VS 2015 doesn't
smeenai added a comment.
I really like the cleanup, but I'm not the biggest fan of taking more Microsoft
dependencies than necessary. I don't see any way around `operator
new`/`operator delete` (since replacement in COFF can only work with a static
library with one function per object file,
EricWF updated this revision to Diff 84613.
EricWF retitled this revision from "Split exception.cpp implementation into
different files for different runtimes" to "Split exception.cpp and new.cpp
implementation into different files for different runtimes".
EricWF edited the summary of this
14 matches
Mail list logo
