[PATCH] D30268: Avoid copy of __atoms when char_type is char

[Eric Fiselier via Phabricator via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM.

Could you please add the benchmark under `libcxx/benchmarks`.

Could you also make sure that the existing tests (under `test/std`) for number parsing have sufficient test coverages for the possible formats it might need to parse? Just to ensure this change isn't missing anything (or the old change isn't)

Once again sorry for the massive delay. `<locale>` patches are tricky, and I barely understand `<locale>` as is.



================
Comment at: libcxx/include/__config:79
 #define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
+
+#define _LIBCPP_ABI_OPTIMIZED_LOCALE
----------------
Could you make the macro name slightly more descriptive, in case we have further optimizations to locale :-)

Also please add a short comment describing the change.


================
Comment at: libcxx/include/locale:969
+#else
+    char_type __atoms[26];
     string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
----------------
Could you lift the `26` into a `const int __atoms_size = 42` that's shared between both `#if` branches?


https://reviews.llvm.org/D30268