The vcruntime headers are hairy and clash with both libc++ headers
themselves and other libraries. libc++ normally deals with the clashes
by deferring to the vcruntime headers and silencing its own definitions,
but for clients which don't want to depend on vcruntime headers, it's
desirable to support the opposite, i.e. have libc++ provide its own
definitions.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Why do we expect the tests to fail without vcruntime headers?
src/support/runtime/exception_msvc.ipp | ||
---|---|---|
31 ↗ | (On Diff #117607) | Really? clang-format removes the space there? typedef void (__CRTDECL * terminate_handler)(void); seems so much more readable. |
msvcrt's implementations of the various new/delete operators call __scrt_throw_std_bad_alloc and __scrt_throw_std_bad_array_new_length, which are found in throw_bad_alloc.obj inside msvcrt.lib. That object file also contains the definitions of various exception methods (e.g. std::exception::~exception) which libc++ will also define for _LIBCPP_NO_VCRUNTIME, leading to symbol conflicts between libc++ and msvcrt. Consequently, _LIBCPP_NO_VCRUNTIME must also use libc++'s definitions of the new/delete operators rather than msvcrt's.
The failing tests check indirect replacement, e.g. replacing operator new, calling operator new[], and expecting the replaced operator new to be called. That would have worked with msvcrt's new/delete operators, since they're statically linked into the executable and therefore their operator new[] can call the replaced operator new, but it won't work with libc++'s new/delete operators, since libc++'s operator new[] is bound to its own operator new. (It would work as expected if the user replaced operator new[] in addition to operator new.) That's unfortunate, but it's also a pretty rare scenario.
On the plus side, the following tests only pass with _LIBCPP_NO_VCRUNTIME. I believe the change in src/new.cpp to include support/runtime/new_handler_fallback.ipp should actually not be limited to _LIBCPP_NO_VCRUNTIME; I'll investigate that in a follow-up.
std/language.support/support.dynamic/alloc.errors/set.new.handler/get_new_handler.pass.cpp std/language.support/support.dynamic/alloc.errors/set.new.handler/set_new_handler.pass.cpp std/language.support/support.dynamic/new.delete/new.delete.array/new_array.pass.cpp std/language.support/support.dynamic/new.delete/new.delete.array/new_array_nothrow.pass.cpp std/language.support/support.dynamic/new.delete/new.delete.single/new.pass.cpp std/language.support/support.dynamic/new.delete/new.delete.single/new_nothrow.pass.cpp
src/support/runtime/exception_msvc.ipp | ||
---|---|---|
31 ↗ | (On Diff #117607) | IIRC it's an issue with having a macro in a function pointer (__CRTDECL) in this case. I'll adjust the spacing manually and file a clang-format bug. |
src/support/runtime/exception_msvc.ipp | ||
---|---|---|
31 ↗ | (On Diff #117607) |
I spoke to @EricWF on IRC last week and got his approval to commit this without review if no one had gotten to it in a couple of days, so I'm going ahead with that.