Skip to content

[libc++] Fix C++23 standard modules when using with clang-cl on Windows #148992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+104 −11
Open

[libc++] Fix C++23 standard modules when using with clang-cl on Windows #148992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed2c4e4
fix libc++ std modules compilation on Windows using clang-cl
siradam7th Jul 15, 2025
b6bf097
fix python file encoding error on Windows
siradam7th Jul 20, 2025
4e7a4c9
add missing functions defined as inline, remove '_BUILD_STD_MODULE'
siradam7th Jul 20, 2025
bf7ca14
remove 'FIXME' for 'get_new_handler()' test
siradam7th Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions libcxx/include/__new/new_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

#if defined(_LIBCPP_ABI_VCRUNTIME)
# include <new.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the MSVC STL get their own definition for get_new_handler()? It seems really brittle to include <new.h> but have our definition for get_new_handler(). We should either declare all of them (set_new_handler(), the new_handler alias, etc) or rely on <new.h> to provide them, but not a mixture of both.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSVC STL just (additionally) declares it in <new>. The declaration is not available from <new.h> or <vcruntime_new.h>.

// <new.h> does not define 'get_new_handler'
// which makes the 'std' module build fail, this fixes it
namespace std {
_LIBCPP_EXPORTED_FROM_ABI new_handler get_new_handler() _NOEXCEPT;
}
#else
_LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD
typedef void (*new_handler)();
Expand Down
46 changes: 44 additions & 2 deletions libcxx/modules/std.compat/ctime.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,57 @@ export {

using ::timespec _LIBCPP_USING_IF_EXISTS;
using ::tm _LIBCPP_USING_IF_EXISTS;

using ::asctime _LIBCPP_USING_IF_EXISTS;
using ::clock _LIBCPP_USING_IF_EXISTS;
using ::strftime _LIBCPP_USING_IF_EXISTS;

#ifndef _LIBCPP_ABI_VCRUNTIME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you simply leaving these functions missing when using UCRT?

Also, it seem that we should modify this line libcxx/utils/libcxx/test/features.py to enable the tests:

llvm-project/libcxx/utils/libcxx/test/features.py

Line 330 in 968d38d

or ("_WIN32" in compilerMacros(cfg) and not _mingwSupportsModules(cfg))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking into it, however it seems that moving the definition '_BUILD_STD_MODULE' triggered some other weird errors, I'll do more testing tomorrow and see...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, these lines should be removed. Now get_new_handler.pass.cpp should always pass.

llvm-project/libcxx/test/std/language.support/support.dynamic/alloc.errors/set.new.handler/get_new_handler.pass.cpp

Lines 11 to 17 in 653872f

// FIXME: When libc++ is linked against vcruntime (i.e. the default config in
// MSVC mode), the declarations of std::set_new_handler and std::get_new_handler
// are provided by vcruntime/UCRT's new.h. However, that header actually only
// declares set_new_handler - it's missing a declaration of get_new_handler.
// XFAIL: msvc && stdlib=libc++

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like just defining the macro is not enough, I have resorted to defining the actual functions as inline, I'll push the changes once I made sure that everything works with both std and std.compact.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _LIBCPP_USING_IF_EXISTS attribute is intended to make things work when the declarations are not available, and avoid precisely these #ifdef blocks. What's the issue here?

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Jul 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue on these <ctime> functions is that UCRT incorrectly adds static to them, so we can't directly export them. At this time we might simply skip export for them to make the standard modules buildable. MSVC STL chooses to reimplement them in the std namespace as a workaround.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it seem that we should modify this line libcxx/utils/libcxx/test/features.py to enable the tests:

llvm-project/libcxx/utils/libcxx/test/features.py

Line 330 in 968d38d

or ("_WIN32" in compilerMacros(cfg) and not _mingwSupportsModules(cfg))

Is just removing that line enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can try this:

or ("_WIN32" in compilerMacros(cfg) and "__MINGW32__" in compilerMacros(cfg) and not _mingwSupportsModules(cfg))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with how the tests are run/generated yet, I'll see...

using ::ctime _LIBCPP_USING_IF_EXISTS;
using ::difftime _LIBCPP_USING_IF_EXISTS;
using ::gmtime _LIBCPP_USING_IF_EXISTS;
using ::localtime _LIBCPP_USING_IF_EXISTS;
using ::mktime _LIBCPP_USING_IF_EXISTS;
using ::strftime _LIBCPP_USING_IF_EXISTS;
using ::time _LIBCPP_USING_IF_EXISTS;
using ::timespec_get _LIBCPP_USING_IF_EXISTS;
#endif

// A workaround for UCRT because it defines these functions
// as static and that causes the error "internal linkage cannot be exported"
#ifdef _LIBCPP_ABI_VCRUNTIME

template <int = 0>
inline char* __CRTDECL ctime(_In_ time_t const* const _Time) noexcept {
return _ctime64(_Time);
}

template <int = 0>
inline double __CRTDECL difftime(_In_ time_t const _Time1, _In_ time_t const _Time2) noexcept {
return _difftime64(_Time1, _Time2);
}

template <int = 0>
inline tm* __CRTDECL gmtime(_In_ time_t const* const _Time) noexcept {
return _gmtime64(_Time);
}

template <int = 0>
inline tm* __CRTDECL localtime(_In_ time_t const* const _Time) noexcept {
return _localtime64(_Time);
}

template <int = 0>
inline time_t __CRTDECL mktime(_Inout_ tm* const _Tm) noexcept {
return _mktime64(_Tm);
}

template <int = 0>
inline time_t __CRTDECL time(_Out_opt_ time_t* const _Time) noexcept {
return _time64(_Time);
}

template <int = 0>
inline int __CRTDECL timespec_get(_Out_ timespec* const _Ts, _In_ int const _Base) noexcept {
return _timespec64_get(reinterpret_cast<_timespec64*>(_Ts), _Base);
}
#endif
} // export
45 changes: 44 additions & 1 deletion libcxx/modules/std/ctime.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,55 @@ export namespace std {

using std::asctime _LIBCPP_USING_IF_EXISTS;
using std::clock _LIBCPP_USING_IF_EXISTS;
using std::strftime _LIBCPP_USING_IF_EXISTS;

#ifndef _LIBCPP_ABI_VCRUNTIME
using std::ctime _LIBCPP_USING_IF_EXISTS;
using std::difftime _LIBCPP_USING_IF_EXISTS;
using std::gmtime _LIBCPP_USING_IF_EXISTS;
using std::localtime _LIBCPP_USING_IF_EXISTS;
using std::mktime _LIBCPP_USING_IF_EXISTS;
using std::strftime _LIBCPP_USING_IF_EXISTS;
using std::time _LIBCPP_USING_IF_EXISTS;
using std::timespec_get _LIBCPP_USING_IF_EXISTS;
#endif

// A workaround for UCRT because it defines these functions
// as static and that causes the error "internal linkage cannot be exported"
#ifdef _LIBCPP_ABI_VCRUNTIME

template <int = 0>
inline char* __CRTDECL ctime(_In_ time_t const* const _Time) noexcept {
return _ctime64(_Time);
}

template <int = 0>
inline double __CRTDECL difftime(_In_ time_t const _Time1, _In_ time_t const _Time2) noexcept {
return _difftime64(_Time1, _Time2);
}

template <int = 0>
inline tm* __CRTDECL gmtime(_In_ time_t const* const _Time) noexcept {
return _gmtime64(_Time);
}

template <int = 0>
inline tm* __CRTDECL localtime(_In_ time_t const* const _Time) noexcept {
return _localtime64(_Time);
}

template <int = 0>
inline time_t __CRTDECL mktime(_Inout_ tm* const _Tm) noexcept {
return _mktime64(_Tm);
}

template <int = 0>
inline time_t __CRTDECL time(_Out_opt_ time_t* const _Time) noexcept {
return _time64(_Time);
}

template <int = 0>
inline int __CRTDECL timespec_get(_Out_ timespec* const _Ts, _In_ int const _Base) noexcept {
return _timespec64_get(reinterpret_cast<_timespec64*>(_Ts), _Base);
}
#endif
} // namespace std
6 changes: 0 additions & 6 deletions ...td/language.support/support.dynamic/alloc.errors/set.new.handler/get_new_handler.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@

// test get_new_handler

// FIXME: When libc++ is linked against vcruntime (i.e. the default config in
// MSVC mode), the declarations of std::set_new_handler and std::get_new_handler
// are provided by vcruntime/UCRT's new.h. However, that header actually only
// declares set_new_handler - it's missing a declaration of get_new_handler.

// XFAIL: msvc && stdlib=libc++

#include <new>
#include <cassert>
Expand Down
13 changes: 11 additions & 2 deletions libcxx/utils/generate_libcxx_cppm_in.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@
import os.path
import sys

from libcxx.header_information import module_c_headers, module_headers, header_restrictions, headers_not_available, libcxx_root
from libcxx.header_information import (
module_c_headers,
module_headers,
header_restrictions,
headers_not_available,
libcxx_root,
)


def write_file(module):
with open(libcxx_root / "modules" / f"{module}.cppm.in", "w") as module_cpp_in:
with open(
libcxx_root / "modules" / f"{module}.cppm.in", "w", encoding="utf-8"
) as module_cpp_in:
module_cpp_in.write(
"""\
// -*- C++ -*-
Expand All @@ -25,6 +33,7 @@ def write_file(module):
//
//===----------------------------------------------------------------------===//


// WARNING, this entire header is generated by
// utils/generate_libcxx_cppm_in.py
// DO NOT MODIFY!
Expand Down