Other OSes may have different mappings from error number to message.
This creates a system to allow new platforms to define their own
mappings.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| libc/src/__support/StringUtil/CMakeLists.txt | ||
|---|---|---|
| 7 | I don't think we should just return when a platform directory is missing. See below. | |
| 11 | This should be conditional: if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
add_subdirectory(${LIBC_TARGET_OS})
endif() | |
| 21 | This should be added conditionally I think: list(APPEND error_to_string_deps
libc.src.errno.errno
libc.src.__support.CPP.span
libc.src.__support.CPP.string_view
libc.src.__support.CPP.stringstream
libc.src.__support.integer_to_string
)
if(TARGET libc.src.__support.StringUtil.${LIBC_TARGET_OS}.strerror_map)
list(APPEND error_to_string_deps libc.src.__support.StringUtil.${LIBC_TARGET_OS}.strerror_map)
else()
list(APPEND error_to_string_deps <target for the default error map - see below>)
endif() | |
| 37 | Ditto. | |
| libc/src/__support/StringUtil/error_to_string.cpp | ||
| 20 | s/__unix__/__linux__ ? | |
| 21 | Instead of expecting everything to be listed in a platform specific table, we can have three kinds of tables:
To get the full Linux table for example, you will have to merge all the above three tables. You likely have to use cpp::array for the tables to get this to work at compile-time. | |
Update the string mapping to support concatenating arrays at compile time.
Simplify the cmake.
Mostly LGTM but I suggested some rearrangement of what you already have to improve modularity.
| libc/src/__support/StringUtil/error_to_string.cpp | ||
|---|---|---|
| 21 | I prefer an organization like this:
inline constexpr ErrorTable<...> STDC_ERRORS = {
...
};
inline constexpr ErrorTable<...> POSIX_EXTENSION_ERRORS = {
...
};
#include "stdc_errors.h"
#include "posix_errros.h"
namespace __llvm_libc {
inline constexpr ErrorTable<...> LINUX_EXTENSION_ERRORS = {
...
};
} // namespace __llvm_libc
#ifdef __linux__ #include "linux/error_table.h" inline constexpr ErrorTable<...> PLATFORM_ERRORS = STDC_ERRORS + POSIX_EXTENSION_ERRORS + LINUX_EXTENSION_ERRORS; #else #include "stdc_error_table.h" inline constexpr ErrorTable<...> PLATFORM_ERRORS = STDC_ERRORS; #endif In this file, all you have to do is include error_table.h and operate on PLATFORM_ERRORS. | |
| libc/src/__support/StringUtil/message_mapper.h | ||
| 32 | Do we need a special class? Can this work: template <size_t N>
using ErrorTable = cpp::array<MsgMapping, N>;
template <size_t N1, size_t N2>
constexpr ErrorTable<N1 + N2> operator+(const ErrorTable &t1, const ErrorTable &t2) {
ErrorTable<N1 + N2> res{};
for (std::size_t i = 0; i < N1; ++i)
res[i] = t1[i];
for (std::size_t i = 0; i < N2; ++i)
res[N1 + i] = t2[i];
return res;
} | |
subdivide the files more and remove unnecessary custom class.
| libc/src/__support/StringUtil/error_to_string.cpp | ||
|---|---|---|
| 21 | I moved these files into a subdirectory for clarity, but otherwise this is laid out as described. | |
| libc/src/__support/StringUtil/message_mapper.h | ||
| 32 | Interesting, this does work, but when I tried adding operator+ to cpp::array directly it complained about the result not being constexpr. | |
| libc/src/__support/StringUtil/CMakeLists.txt | ||
|---|---|---|
| 11 | We are moving to giving directories also lower case names. So, may be just tables for the nested directory? | |
| libc/src/__support/StringUtil/TableMappings/error_table.h | ||
| 26 ↗ | (On Diff #514742) | Lets not add POSIX_ERRORS for the default case. |
| libc/src/__support/StringUtil/TableMappings/linux/error_table.h | ||
| 19 ↗ | (On Diff #514742) | constexpr tables declared in header files should be inline constexpr .... |
| libc/src/__support/StringUtil/error_to_string.cpp | ||
| 33 | Remove? | |
I don't think we should just return when a platform directory is missing. See below.