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

[Sean via Phabricator via libcxx-commits]

SeanP added inline comments.


================
Comment at: libcxx/include/__locale:571
 
+    static bool isbasic(char_type __c)
+    {
----------------
SeanP wrote:
> Rather than making this a static member function, I suggest creating an anonymous namespace outside the class and putting the function in there and as an inline function.  You can then get rid of the duplicate definition in the other class.
> 
> I suspect to that for MVS we need to have one check for EBCDIC ([0,255]) and another for ASCII ([0,127]).  
Nancy reminded me off-line that the char traits tables for MVS work on the range 0-255 for both ASCII and EBCDIC mode on MVS.  The code, as is, is good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118851