Re: [boost] [locale] Review part 1: headers

Subject: Re: [boost] [locale] Review part 1: headers
From: Mathias Gaunard (mathias.gaunard_at_[hidden])
Date: 2011-04-09 06:20:13

On 09/04/2011 11:50, Artyom wrote:

> I don't think neither ICU nor Boost supports
> non-32 bit platform.
>
> I understand that strictly speaking it is non-standard
> but de-facto there are no problems with this and this
> assumption is quite safe.

Since this assumption arbitrarily limits the library to particular 
platforms and it is trivial to avoid relying it, I see no point in doing so.

>
> I can change this to uint32_t
>
>> line 131: Please be consistent about using unsigned vs. uint32_t.
>
> Good point
>
>> line 140: shouldn't offset be std::size_t?
>>    Why should the size of the string be limited
>>    to 4GB?  std::size_t is guaranteed to be
>>    large enough to handle anything that can
>>    be stored in memory.
>>
>
> You are right, I'll fix this.
>
> However, two points:
>
> 1. I already know compilers were size_t != sizeof(void *)
>     so actually it must be uintptr_t... but it is not in C++03
>     but rather in C99. (it is in C++0x)

1) You work with contiguous memory, so size_t is the correct one, not 
uintptr_t

2) uintptr_t should ideally be available from boost/cstdint.hpp (it 
doesn't seem to be ATM, that's a bug)

3) I don't know of compilers where those are different, mind saying 
which ones exhibit this?

> 2. There is no reason on planet earth you need to
>     create boundary mapping for text>= 4G!
>
>     Even entire War&  Peace book takes about 3MB.
>
>     So if you do this you are probably doing
>     something very-very wrong.
>
>     But this is different story.

Another unnecessary arbitrary limit.

Why would you want to prevent people from treating very large data with 
your library?

On the Internet, there is text much bigger than War & Peace, even if not 
necessarily in the form of books.

>
>
>> Why do you have explicit specializations of
>> boundary_indexing?  They're all the same.
>> The compiler is perfectly capable of instantiating
>> them all from a single definition.
>>
>
> Ok this is something that you'll see all over the Boost.Locale
> code when it comes to deal with facets generations.
>
> It is in order to support (brain damaged) DLL platform.

Both MSVC and GCC support extern template, which is also a C++0x feature.

> Yes... However all supported platforms are at least 32bits.
>
> There is one small points. As far as I remember I
> always used native types over strict types in the interfaces
> (function calls) because if it is changed between
> small releases (like stdint.h found) it would not
> break binary compatibility so native types (as long
> as they good enough) are better alternative.
>
> (We are talking about C++ compilers that does not
> handle C99 stdint well)

Boost has a portable stdint equivalent.

In any case, you shouldn't even use uint32_t, but uint_least32_t, or 
better yet, char32_t if it is available.

boost/cuchar.cpp in my Unicode library shows the right way to define the 
types Unicode needs to work with.