[libcxx-commits] [PATCH] D118851: [SystemZ]:[z/OS]:[libcxx]: fix isascii function to work for z/OS

[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/src/locale.cpp:1029
+      *low =
+          __is_valid_in_encoding(*low) ? static_cast<char>(__classic_upper_table()[static_cast<size_t>(*low)]) : *low;
 #else
----------------
Changes like this one make me nervous. The original purpose of `isascii(x)` seems to have been a bounds check: it says "yes, `x` is in the range 0 to 127 inclusive." That doesn't really have anything to do with the ASCII versus EBCDIC question — hence the mass renaming to `__is_valid_in_encoding`, on which I'm neutral. However, it is //very important// to all of these table lookups: If `x` isn't in the range 0 to 127, e.g. if x is `-1` or `10000`, then these lookups will have UB.

Can you explain why you want to make this change? Specifically, what behaves wrongly on your platform if we //don't do any of this?//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118851