[cfe-dev] [libc++] xlocale usage portability issues

[aep]

> Is there a flag that can be used to identify your port instead of
> just putting it all under "#else"?  Your very first change:

unfortunately not. but it might not be needed for anything but the 
xlocale.h
I am right now patching musl to include the nessesary _l functions 
instead.
David Chisnall has convinced me, it is worth the effort.


>> -# include <xlocale.h>
>> +# include <locale.h>
>>  #endif  // _WIN32
>
> breaks OS X and FreeBSD.


sorry that wasn't intended as a real patch. I don't quite know how to 
handle the missing xlocale.h case yet. maybe #ifdef __GLIBC__ it

>
>
>>
>>  #pragma GCC system_header
>> @@ -235,11 +235,11 @@ collate<_CharT>::do_hash(const char_type* lo,
>> const char_type* hi) const
>>  {
>>      size_t h = 0;
>>      const size_t sr = __CHAR_BIT__ * sizeof(size_t) - 8;
>> -    const size_t mask = size_t(0xF) << (sr + 4);
>> +    const size_t cmask = size_t(0xF) << (sr + 4);
>>      for(const char_type* p = lo; p != hi; ++p)
>>      {
>>          h = (h << 4) + *p;
>> -        size_t g = h & mask;
>> +        size_t g = h & cmask;
>>          h ^= g | (g >> sr);
>>      }
>>      return static_cast<long>(h);
>
> Why does mask need to be renamed to cmask?

good question, clang throws a weird error otherwise:

/home/aep/proj/evprojects/heresy/libcpp/evocation_build/build/s/
include/__locale:238:23: error: expected unqualified-id
      const size_t mask = size_t(0xF) << (sr + 4);
                        ^

(it points at the = just in case my mail client screws up the indend)


>
>
>> @@ -330,7 +330,7 @@ public:
>>      static const mask punct  = _PUNCT;
>>      static const mask xdigit = _HEX;
>>      static const mask blank  = _BLANK;
>> -#else  // __GLIBC__ || _WIN32
>> +#elif (__APPLE__ || __FreeBSD__)
>>  #if __APPLE__
>>      typedef __uint32_t mask;
>>  #elif __FreeBSD__
>> @@ -346,7 +346,19 @@ public:
>>      static const mask punct  = _CTYPE_P;
>>      static const mask xdigit = _CTYPE_X;
>>      static const mask blank  = _CTYPE_B;
>> -#endif  // __GLIBC__ || _WIN32
>> +#else  // __GLIBC__ || _WIN32 || __APPLE__ || __FreeBSD__
>> +    typedef unsigned long mask;
>> +    static const mask space  = 1<<0;
>> +    static const mask print  = 1<<1;
>> +    static const mask cntrl  = 1<<2;
>> +    static const mask upper  = 1<<3;
>> +    static const mask lower  = 1<<4;
>> +    static const mask alpha  = 1<<5;
>> +    static const mask digit  = 1<<6;
>> +    static const mask punct  = 1<<7;
>> +    static const mask xdigit = 1<<8;
>> +    static const mask blank  = 1<<9;
>> +#endif  // __GLIBC__ || _WIN32 || __APPLE__ || __FreeBSD__
>>      static const mask alnum  = alpha | digit;
>>      static const mask graph  = alnum | punct;
>
> This part looks ok, but we're getting enough platforms in here we
> should probably do away with the generic "else" branch (including for
> Apple and FreeBSD as you suggest above).


right, if you want i can remove the Apple and Freebsd branches, but i 
am unsure about the implications.
David said there was some good reason for using the libc's types here. 
But i don't it from my libc's POV.

>
> Thanks for working on this!

thanks for libc++ :)