[libcxx-commits] [PATCH] D120547: [libcxx] [test] Fix the monetary locale curr_symbol test on Windows

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

Seems reasonable to me. Some pre-existing nitpicks.



================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.moneypunct.byname/curr_symbol.pass.cpp:9
 //
 // XFAIL: darwin
 //
----------------
Any chance this passes on Apple now? If not, do you mind adding a one-line comment here since presumably you've already dug into it a little bit?


================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.moneypunct.byname/curr_symbol.pass.cpp:162
         else
           assert(f.curr_symbol() == L" \x440\x443\x431");
+#elif defined(_WIN32)
----------------
Pre-existing: it sure would be nice if this could be changed to `L" \u0440\u0443\u0431"` for clarity. I'm like 50% sure those mean the same thing. :)


================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.moneypunct.byname/curr_symbol.pass.cpp:178-182
+#ifdef _WIN32
+        assert(f.curr_symbol() == "\xC2\xA5");
+#else
         assert(f.curr_symbol() == "\xEF\xBF\xA5");
+#endif
----------------
Could these be `"\u00A5"` and `"\uFFE5"` for clarity?


================
Comment at: libcxx/test/std/localization/locale.categories/category.monetary/locale.moneypunct.byname/curr_symbol.pass.cpp:191-195
+#ifdef _WIN32
+        assert(f.curr_symbol() == L"\x00A5");
+#else
         assert(f.curr_symbol() == L"\xFFE5");
+#endif
----------------
I think these should use `\u` not `\x`, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120547