[libc-commits] [PATCH] D147967: [libc] move strerror and strsignal to OS msg maps

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Mostly LGTM but I suggested some rearrangement of what you already have to improve modularity.



================
Comment at: libc/src/__support/StringUtil/error_to_string.cpp:18
+#ifdef __unix__
+#include "src/__support/StringUtil/linux/strerror_map.h"
+#else
----------------
sivachandra wrote:
> Instead of expecting everything to be listed in a platform specific table, we can have three kinds of tables:
> 
> 1. Table of errors specified by the C standard - this will be the fallback table instead of the `#error` you have below.
> 2. Table of POSIX extension errors.
> 3. Table of Linux extension errors.
> 
> 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.
I prefer an organization like this:

1. `stdc_error_table.h` contains:
```
inline constexpr ErrorTable<...> STDC_ERRORS = {
  ...
};
```
2. `posix_error_table.h` contains:
```
inline constexpr ErrorTable<...> POSIX_EXTENSION_ERRORS = {
  ...
};
```
3. `linux/error_table.h` contains:
```
#include "stdc_errors.h"
#include "posix_errros.h"

namespace __llvm_libc {
inline constexpr ErrorTable<...> LINUX_EXTENSION_ERRORS = {
  ...
};
} // namespace __llvm_libc
```
4. `error_table.h` contains:
```
#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`.


================
Comment at: libc/src/__support/StringUtil/message_mapper.h:31
 
+template <class T, size_t N> class ArrayConcat {
+  T Data[N];
----------------
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;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147967/new/

https://reviews.llvm.org/D147967