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

Subject: Re: [boost] [locale] Review part 1: headers
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-04-09 07:38:19

> 
> > 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.
> 

As I told you I can fix it, but practically it does
not give any advantage.

Give me a name of modern compiler that supports C++03
and has sizeof(unsigned) < 4?

> 
> >
> >  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
> 

size_t is used in boost as it is closest way to use something
in C++03 standard that is similar to uintptr_t.

That is way C99 and C++03 created uintptr_t.

In any case as I told I'll change this for better consistency/

> 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?
> 

As I said before I prefer to relay on native types rather
then boost's types especially if it is feasible.

As I told above I'll change this were necessary but
it is just for "code-beauty".

> 
> > 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.
> 

As I told above I'll change it to size_t but...

You don't want to run boundary analysis on such chunks of text
in any case - and if you do it, you are doing something wrong.

> >
> >
> >> 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.
> 

Believe me, it was only feasible solution I've reached after
spending many man hours fighting DLL's, std::locale facets and
both compilers.

(AFAIR MSVC uses specialization as well for its own facets)

So after fighting both compilers and spending days on solving and tuning
this problem I can say this is what should be done.

It requires some good experience with std::locale facets, DLLs
exporting, and having specific tests.

So what I did is unfortunately required and most reliable solution.

> 
> > 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.
> 

I know and use it, see the description above.

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

No, see:

   
http://cppcms.sourceforge.net/boost_locale/html/appendix.html#why_no_special_character_type

In any case I use uint32_t or uint16_t when I do relate to handling
of UTF-16 or UTF-32

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

Same link as above, typedef of uint16_t or uint32_t is unsuitable for 
Boost.Locale
character representation.

See your Unicode library is orthogonal to Boost.Locale, it
handles things very different and has different purpose.

Artyom