[libc++] Fix missing 'get_new_handler()' for Windows builds #150182
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Adam (siradam7th) ChangesThe following PR fixes the missing Fixes: Related PR: Full diff: https://github.com/llvm/llvm-project/pull/150182.diff 2 Files Affected:
diff --git a/libcxx/include/__new/new_handler.h b/libcxx/include/__new/new_handler.h
index 05f4e846c3ef9..8f93578da9f9c 100644
--- a/libcxx/include/__new/new_handler.h
+++ b/libcxx/include/__new/new_handler.h
@@ -15,14 +15,10 @@
# pragma GCC system_header
#endif
-#if defined(_LIBCPP_ABI_VCRUNTIME)
-# include <new.h>
-#else
_LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD
typedef void (*new_handler)();
_LIBCPP_EXPORTED_FROM_ABI new_handler set_new_handler(new_handler) _NOEXCEPT;
_LIBCPP_EXPORTED_FROM_ABI new_handler get_new_handler() _NOEXCEPT;
_LIBCPP_END_UNVERSIONED_NAMESPACE_STD
-#endif // _LIBCPP_ABI_VCRUNTIME
#endif // _LIBCPP___NEW_NEW_HANDLER_H
diff --git a/libcxx/src/new_handler.cpp b/libcxx/src/new_handler.cpp
index 49c21d85f5908..5124cb501c938 100644
--- a/libcxx/src/new_handler.cpp
+++ b/libcxx/src/new_handler.cpp
@@ -11,9 +11,7 @@
#include "include/atomic_support.h"
#if defined(_LIBCPP_ABI_MICROSOFT)
-# if !defined(_LIBCPP_ABI_VCRUNTIME)
-# define _LIBPCPP_DEFINE_NEW_HANDLER
-# endif
+# define _LIBPCPP_DEFINE_NEW_HANDLER
#elif defined(LIBCXX_BUILDING_LIBCXXABI)
// nothing to do, we use the one from libc++abi
#elif defined(LIBCXXRT)
|
|
|
| # if !defined(_LIBCPP_ABI_VCRUNTIME) | ||
| # define _LIBPCPP_DEFINE_NEW_HANDLER | ||
| # endif | ||
| # define _LIBPCPP_DEFINE_NEW_HANDLER |
There was a problem hiding this comment.
This doesn't seem right. The definitions provided __new_handler are dummy and non-conformant. Perhaps we should rely on VCRuntime's functionalities.
There was a problem hiding this comment.
I wonder whether we should include <vcruntime_new.h> instead of <new.h> (in other headers).
There was a problem hiding this comment.
I wonder whether we should include
<vcruntime_new.h>instead of<new.h>(in other headers).
Unfortunately that would increase the scope of this PR, which is why it exists in standalone form rather than with #14899
There was a problem hiding this comment.
This doesn't seem right. The definitions provided
__new_handlerare dummy and non-conformant. Perhaps we should rely on VCRuntime's functionalities.
Care to elaborate? from what I've seen in MS-STL they both have similar implementations.
The following PR fixes the missing
get_new_handler()function for Windows by using the same implementation as other platforms since<new.h>in the UCRT is missing this function and only definesset_new_handler()so it is best if all of them are defined by libc++.Fixes:
Related PR:
clang-clon Windows #148992