[libcxx-commits] [PATCH] D120796: [libcxx] Fix the locale `is` and `scan_is`/`scan_not` tests for Windows

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__locale:457
 # define _LIBCPP_CTYPE_MASK_IS_COMPOSITE_PRINT
+# define _LIBCPP_CTYPE_MASK_IS_COMPOSITE_ALPHA
 #elif defined(__APPLE__) || defined(__FreeBSD__) || defined(__EMSCRIPTEN__) || defined(__NetBSD__)
----------------
This looks like a bugfix, so "Fix the ... tests" seems like the wrong commit summary.
Before this patch, did `f.is(F::upper, L'a')` return `true` instead of `false`? That'd be a good thing to put in the commit message.


================
Comment at: libcxx/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/is_1.pass.cpp:110-112
+            // On Windows, in the "C" locale, \x00DA is interpreted according
+            // to the current code page, where this char can be considered an
+            // uppercase alphabetical character.
----------------
This comment is too vague for my liking. I infer that "can be" means sometimes it is and sometimes it isn't. I'd rather see something very specific, like
```
// On Windows, this depends on the current codepage.
// If the codepage is CP-437, \x00DA is U+250C BOX DRAWINGS LIGHT DOWN AND RIGHT.
// If the codepage is CP-1252, \x00DA is U+00DA CAPITAL LETTER U WITH ACUTE.
```
And then I'm not actually sure why the non-Windows behavior claims that U+00DA is //not// alpha+upper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120796