PR 13631 Problems in messages

[François Dumont]

Hi

     Here is another proposal that consider all your remarks except one. 
I finally prefer to go with std::vector of pointers. Dynamically 
allocating Catalog_info allow to avoid numerous copies of locale when we 
find this pointer from the catalog info.


On 28/11/2014 11:49, Jonathan Wakely wrote:
> + class Catalogs
>> +  {
>> +  public:
>> +    typedef messages_base::catalog catalog_id;
>> +
>> +    struct catalog_info
>> +    {
>> +      catalog_info()
>> +      { }
>> +
>> +      catalog_info(catalog_id __id, const char* __domain, locale __loc)
>> +    : _M_id(__id), _M_domain(__domain), _M_locale(__loc)
>> +      { }
>> +
>
> I don't really like that this type doesn't own _M_domain and it has to
> be freed by Catalog, but without a move constructor (which requires
> C++11) it would be awkward to set _M_domain=0 after copy constructing
> a new catalog_info.
>
> So it's OK like this.

I don't liked it neither so now it is a usual std::string.

> + result_type
>> +    _M_get(catalog_id __c) const
>> +    {
>> +      __gnu_cxx::__scoped_lock lock(_M_mutex);
>> +
>> +      const catalog_info* __entry =
>> +    lower_bound(_M_map, _M_map + _M_nb_entry, __c, _Comp());
>> +      if (__entry != _M_map + _M_nb_entry && __entry->_M_id == __c)
>> +    return result_type(__entry->_M_domain, __entry->_M_locale);
>> +      return result_type(0, locale());
>
> I assume the second return was just for testing, it should be removed.

I don't get this one. I still need to return something when I can't find 
the catalog so the 2 return statements.

> Index: config/locale/gnu/messages_members.h
>> ===================================================================
>> --- config/locale/gnu/messages_members.h    (revision 218027)
>> +++ config/locale/gnu/messages_members.h    (working copy)
>> @@ -83,22 +83,6 @@
>>       _S_destroy_c_locale(_M_c_locale_messages);     }
>>
>> -  template<typename _CharT>
>> -    typename messages<_CharT>::catalog - 
>> messages<_CharT>::do_open(const basic_string<char>& __s, 
>> -                  const locale&) const
>> -    { -      // No error checking is done, assume the catalog exists 
>> and can
>> -      // be used.
>> -      textdomain(__s.c_str());
>> -      return 0;
>> -    }
>> -
>> -  template<typename _CharT>
>> -    void    -    messages<_CharT>::do_close(catalog) const -    { }
>> -
>>    // messages_byname
>>    template<typename _CharT>
>>      messages_byname<_CharT>::messages_byname(const char* __s, size_t 
>> __refs)
>
> Unless I'm misreading this patch, you've removed the definitions of
> messages<_CharT>::do_open() and messages<_CharT>::do_close() for the
> primary template. They would stil be needed if users instantiate e.g.
> messages<char16_t> or messages<signed char>.

Yes but do you confirm that it is already the same problem with do_get ?

In my opinion we could provide template implementations of all those 
methods relying on codecvt<_CharT, char, mbstate_t>, even for do_get. 
But in this case some implementation details will be exposed in the 
header files and additional symbols will have to be exported I think.

In fact I have already started doing something like that but then start 
facing issue accessing nl_langinfo_l. Shall I go further and provide a 
patch doing this ?

François

-------------- next part --------------
A non-text attachment was scrubbed...
Name: messages.patch
Type: text/x-patch
Size: 14632 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20141201/4ab9c80a/attachment.bin>