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.