[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