[libcxx-commits] [PATCH] D111265: [libc++] Add an option to disable wide character support in libc++

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Somehow two of my comments weren't submitted and were lost. So here they are again.



================
Comment at: libcxx/src/locale.cpp:4573
   }
+#endif // _LIBCPP_HAS_NO_WIDE_CHARACTERS
   _LIBCPP_UNREACHABLE();
----------------
I we should `return false` instead of reaching `_LIBCPP_UNREACHABLE();`.

When I understand the code correctly this is only an issue when the supplied "string" contains more than one character. This basically means `numpunct`'s `decimal_point` or `thousands_sep` isn't a single character. I think it would suffice when adding this `return` and handle the returned `false` in the callers. 

When vendors report that this doesn't work properly we can discuss with them what a better solution would be, for example using a lookup table to translate their "string" to a `char`. For now that seems over engineering.


================
Comment at: libcxx/src/locale.cpp:4665
         lconv* lc = __libcpp_localeconv_l(loc.get());
         checked_string_to_char_convert(__decimal_point_, lc->decimal_point,
                                        loc.get());
----------------
I think we should validate the return value here and two lines below.
Something like done in `moneypunct_byname<char, false>`
```
    if (!checked_string_to_char_convert(__decimal_point_,
                                        lc->mon_decimal_point,
                                        loc.get()))
      __decimal_point_ = base::do_decimal_point();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111265