[libcxx-commits] [PATCH] D120889: [libcxx] [test] Fix the get/put long_double_zh_CN tests on Windows
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Mar 3 08:00:52 PST 2022
Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.
================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.money.get/locale.money.get.members/get_long_double_zh_CN.pass.cpp:71-77
+#ifdef _WIN32
+ std::string curr_symbol = "\u00A5";
+ std::string curr_text = "CNY";
+#else
+ std::string curr_symbol = "\uFFE5";
+ std::string curr_text = "CNY ";
#endif
----------------
Naming nit: By the time I saw it again below, I'd forgotten that `curr_text` stood for `currency_name` and not `current_string`. I suggest spelling out `currency_{symbol,name}`, or even better, refactoring into a helper function as you suggest in your PR summary.
As in the previous PR, using a `MultiStringType` helper will eliminate the need for `curr_x` and `w_curr_x` to coexist.
================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.money.put/locale.money.put.members/put_long_double_zh_CN.pass.cpp:82
+ std::string curr_text = "CNY ";
+ std::string curr_pad20_text = " ";
+#endif
----------------
I suggest that this could be expressed as `std::string(7 - curr_text.size(), ' ')` and then inlined everywhere it's used. The name `pad20` is awkward in that this string definitely isn't 20 bytes long. :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120889/new/
https://reviews.llvm.org/D120889
More information about the libcxx-commits
mailing list