libcxx's modules build will crash #58716
Comments
EugeneZelenko
added
clang:modules
|
@llvm/issue-subscribers-clang-codegen |
|
@llvm/issue-subscribers-clang-modules |
|
Ha! I encountered this when trying to include // mymodule.cppm
module;
#include "fail.h"
export module mymodule;
// fail.h
#include <string>
#include <format>
auto this_fails() -> void {
std::string buffer;
std::format_to(std::back_inserter(buffer), "{}", "hello");
}Precompilation works. The failure occurs during It fails during the |
|
@aaronmondal Thanks for reporting. And I get a minimal reproducer from your reproducer: The crash goes aways if we don't mark the constructor of |
|
@ChuanqiXu9 Ah yes this seems to be an issue with consteval. One other occurence of this error is at The following hacky workaround patch makes at least the spdlog build pass. diff --git a/libcxx/include/__chrono/statically_widen.h b/libcxx/include/__chrono/statically_widen.h
index dd12c3f5020e..6d29452e765d 100644
--- a/libcxx/include/__chrono/statically_widen.h
+++ b/libcxx/include/__chrono/statically_widen.h
@@ -26,7 +26,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
# ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
template <__fmt_char_type _CharT>
-consteval const _CharT* __statically_widen(const char* __str, const wchar_t* __wstr) {
+constexpr const _CharT* __statically_widen(const char* __str, const wchar_t* __wstr) {
if constexpr (same_as<_CharT, char>)
return __str;
else
@@ -39,7 +39,7 @@ consteval const _CharT* __statically_widen(const char* __str, const wchar_t* __w
// fails for the CI build "No wide characters". This seems like a bug.
// TODO FMT investigate why this is needed.
template <__fmt_char_type _CharT>
-consteval const _CharT* __statically_widen(const char* __str) {
+constexpr const _CharT* __statically_widen(const char* __str) {
return __str;
}
# define _LIBCPP_STATICALLY_WIDEN(_CharT, __str) ::std::__statically_widen<_CharT>(__str)
diff --git a/libcxx/include/__format/format_functions.h b/libcxx/include/__format/format_functions.h
index 2e7925bfbd01..f8a0352b328a 100644
--- a/libcxx/include/__format/format_functions.h
+++ b/libcxx/include/__format/format_functions.h
@@ -335,7 +335,7 @@ template <class _CharT, class... _Args>
struct _LIBCPP_TEMPLATE_VIS basic_format_string {
template <class _Tp>
requires convertible_to<const _Tp&, basic_string_view<_CharT>>
- consteval basic_format_string(const _Tp& __str) : __str_{__str} {
+ constexpr basic_format_string(const _Tp& __str) : __str_{__str} {
__format::__vformat_to(basic_format_parse_context<_CharT>{__str_, sizeof...(_Args)},
_Context{__types_.data(), __handles_.data(), sizeof...(_Args)});
} |
Is there a bug reporting? |
|
Not entirely sure whether that's related. I need to investigate whether it's really a bug not. I just changed the |
|
@mordante Got it.
BTW, do you know if there are any plan about supporting std modules in libcxx? I mean something like https://reviews.llvm.org/D135507. (I'm not selling it. I know its current quality is not good enough). It shows it is not impossible to talk about C++20 Modules in libcxx now. And I want to know if there is anyone who is interested. |
|
I'm definitely interested in modules in libc++. However I'm not sure what the status of C++20 modules in clang is. I think it would be best to discuss this in the libcxx Discord channel and maybe have a short telecon with the interested parties. |
Good idea. I mentioned it in https://discord.com/channels/636084430946959380/636732894974312448/1052403022271094824. I wanted to ping you but I can't find your name there. |
…Helper's type properly Close llvm/llvm-project#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
…Helper's type properly Close llvm/llvm-project#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
…Helper's type properly Close llvm/llvm-project#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
…Helper's type properly Close llvm/llvm-project#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
Close llvm#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
…Helper's type properly Close llvm/llvm-project#58716. Tested with libcxx's modules build. When we read the type of LValuePathSerializationHelper, we didn't read the correct type. We read the element type as its name suggests. But the problem here is that it looks like that both the usage and serialization use its type as the top level type. So here is the mismatch. Actually, the type of LValuePathSerializationHelper is never used after Deserialization without the assertion. So it doesn't matter for the release users. And this patch shouldn't change the behavior too. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D139406
From https://reviews.llvm.org/D134036.
Reproduce:
Crash log:
The text was updated successfully, but these errors were encountered: