Re: Elimitate duplication of get_catalogs in different abi

["François Dumont" <frs.dumont@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On 22/08/2015 14:24, Daniel Krügler wrote:
> 2015-08-21 23:11 GMT+02:00 François Dumont <frs.dumont@gmail.com>:
>> I think I found a better way to handle this problem. It is c++locale.cc
>> that needs to be built with --fimplicit-templates. I even think that the
>> *_cow.cc file do not need this option but as I don't know what is the
>> drawback of this option I kept it. I also explicitely used the file name
>> c++locale.cc even if it is an alias to a configurable source file.  I
>> guess there must be some variable to use no ?
>>
>> With this patch there are 6 additional symbols. I guess I need to
>> declare those in the scripts even if it is for internal library usage,
>> right ?
> I would expect that the new Catalog_info definition either has deleted
> or properly (user-)defined copy constructor and copy assignment
> operator.
>
>
> - Daniel
>
This type is used in C++98 so I need to make those private, not deleted.

With this change, is the patch ok to commit ?

François


[-- Attachment #2: messages_catalogs.patch --]
[-- Type: text/x-patch, Size: 11560 bytes --]

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index fc97cdf..8f9f99a 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1871,6 +1871,14 @@ GLIBCXX_3.4.22 {
     # std::uncaught_exceptions()
     _ZSt19uncaught_exceptionsv;
 
+    # std::Catalogs::*
+    extern "C++"
+    {
+      std::Catalogs::*;
+    };
+
+    _ZNSt6vectorIPSt12Catalog_info*;
+
 } GLIBCXX_3.4.21;
 
 # Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/config/locale/gnu/c++locale_internal.h b/libstdc++-v3/config/locale/gnu/c++locale_internal.h
index f1959d6..7db354c 100644
--- a/libstdc++-v3/config/locale/gnu/c++locale_internal.h
+++ b/libstdc++-v3/config/locale/gnu/c++locale_internal.h
@@ -36,8 +36,13 @@
 #include <cstddef>
 #include <langinfo.h>
 
+#include <vector>
+#include <string.h>	// ::strdup
+
+#include <ext/concurrence.h>
+
 #if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ > 2)
-                                                  
+
 extern "C" __typeof(nl_langinfo_l) __nl_langinfo_l;
 extern "C" __typeof(strcoll_l) __strcoll_l;
 extern "C" __typeof(strftime_l) __strftime_l;
@@ -61,3 +66,55 @@ extern "C" __typeof(wctype_l) __wctype_l;
 #endif 
 
 #endif // GLIBC 2.3 and later
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  struct Catalog_info
+  {
+    Catalog_info(messages_base::catalog __id, const char* __domain,
+		 locale __loc)
+      : _M_id(__id), _M_domain(strdup(__domain)), _M_locale(__loc)
+    { }
+
+    ~Catalog_info()
+    { free(_M_domain); }
+
+    messages_base::catalog _M_id;
+    char* _M_domain;
+    locale _M_locale;
+
+  private:
+    Catalog_info(const Catalog_info&);
+
+    Catalog_info&
+    operator=(const Catalog_info&);
+  };
+
+  class Catalogs
+  {
+  public:
+    Catalogs() : _M_catalog_counter(0) { }
+    ~Catalogs();
+
+    messages_base::catalog
+    _M_add(const char* __domain, locale __l);
+
+    void
+    _M_erase(messages_base::catalog __c);
+
+    const Catalog_info*
+    _M_get(messages_base::catalog __c) const;
+
+  private:
+    mutable __gnu_cxx::__mutex _M_mutex;
+    messages_base::catalog _M_catalog_counter;
+    vector<Catalog_info*> _M_infos;
+  };
+
+  Catalogs&
+  get_catalogs();
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
diff --git a/libstdc++-v3/config/locale/gnu/c_locale.cc b/libstdc++-v3/config/locale/gnu/c_locale.cc
index 4612c64..708af0e 100644
--- a/libstdc++-v3/config/locale/gnu/c_locale.cc
+++ b/libstdc++-v3/config/locale/gnu/c_locale.cc
@@ -31,9 +31,12 @@
 #include <locale>
 #include <stdexcept>
 #include <limits>
+#include <algorithm>
 #include <langinfo.h>
 #include <bits/c++locale_internal.h>
 
+#include <backward/auto_ptr.h>
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -170,6 +173,85 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     return __changed;
   }
 
+  struct _CatalogIdComp
+  {
+    bool
+    operator()(messages_base::catalog __cat, const Catalog_info* __info) const
+    { return __cat < __info->_M_id; }
+
+    bool
+    operator()(const Catalog_info* __info, messages_base::catalog __cat) const
+    { return __info->_M_id < __cat; }
+  };
+
+  Catalogs::~Catalogs()
+  {
+    for (vector<Catalog_info*>::iterator __it = _M_infos.begin();
+	 __it != _M_infos.end(); ++__it)
+      delete *__it;
+  }
+
+  messages_base::catalog
+  Catalogs::_M_add(const char* __domain, locale __l)
+  {
+    __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+    // The counter is not likely to roll unless catalogs keep on being
+    // opened/closed which is consider as an application mistake for the
+    // moment.
+    if (_M_catalog_counter == numeric_limits<messages_base::catalog>::max())
+      return -1;
+
+    auto_ptr<Catalog_info> info(new Catalog_info(_M_catalog_counter++,
+						 __domain, __l));
+
+    // Check if we managed to allocate memory for domain.
+    if (!info->_M_domain)
+      return -1;
+
+    _M_infos.push_back(info.get());
+    return info.release()->_M_id;
+  }
+
+  void
+  Catalogs::_M_erase(messages_base::catalog __c)
+  {
+    __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+    vector<Catalog_info*>::iterator __res =
+      lower_bound(_M_infos.begin(), _M_infos.end(), __c, _CatalogIdComp());
+    if (__res == _M_infos.end() || (*__res)->_M_id != __c)
+      return;
+
+    delete *__res;
+    _M_infos.erase(__res);
+
+    // Just in case closed catalog was the last open.
+    if (__c == _M_catalog_counter - 1)
+      --_M_catalog_counter;
+  }
+
+  const Catalog_info*
+  Catalogs::_M_get(messages_base::catalog __c) const
+  {
+    __gnu_cxx::__scoped_lock lock(_M_mutex);
+
+    vector<Catalog_info*>::const_iterator __res =
+      lower_bound(_M_infos.begin(), _M_infos.end(), __c, _CatalogIdComp());
+
+    if (__res != _M_infos.end() && (*__res)->_M_id == __c)
+      return *__res;
+
+    return 0;
+  }
+
+  Catalogs&
+  get_catalogs()
+  {
+    static Catalogs __catalogs;
+    return __catalogs;
+  }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
@@ -211,3 +293,4 @@ _GLIBCXX_END_NAMESPACE_VERSION
   extern "C" void ldbl (void) __attribute__ ((alias (#dbl)))
 _GLIBCXX_LDBL_COMPAT(_ZSt14__convert_to_vIdEvPKcRT_RSt12_Ios_IostateRKP15__locale_struct, _ZSt14__convert_to_vIeEvPKcRT_RSt12_Ios_IostateRKP15__locale_struct);
 #endif // _GLIBCXX_LONG_DOUBLE_COMPAT
+
diff --git a/libstdc++-v3/config/locale/gnu/messages_members.cc b/libstdc++-v3/config/locale/gnu/messages_members.cc
index 2e6122d..90f4b8d 100644
--- a/libstdc++-v3/config/locale/gnu/messages_members.cc
+++ b/libstdc++-v3/config/locale/gnu/messages_members.cc
@@ -31,115 +31,13 @@
 #include <locale>
 #include <bits/c++locale_internal.h>
 
-#include <limits>
-#include <algorithm>
-#include <vector>
 #include <cstdlib>	// std::free
 #include <string.h>	// ::strdup
 
-#include <backward/auto_ptr.h>
-#include <ext/concurrence.h>
-
 namespace
 {
   using namespace std;
 
-  typedef messages_base::catalog catalog;
-
-  struct Catalog_info
-    {
-    Catalog_info(catalog __id, const string& __domain, locale __loc)
-      : _M_id(__id), _M_domain(__domain), _M_locale(__loc)
-    { }
-
-    catalog _M_id;
-    string _M_domain;
-    locale _M_locale;
-  };
-
-  class Catalogs
-  {
-  public:
-    Catalogs() : _M_catalog_counter(0) { }
-
-    ~Catalogs()
-    {
-      for (vector<Catalog_info*>::iterator __it = _M_infos.begin();
-	   __it != _M_infos.end(); ++__it)
-	delete *__it;
-    }
-
-    catalog
-    _M_add(const string& __domain, locale __l)
-    {
-      __gnu_cxx::__scoped_lock lock(_M_mutex);
-
-      // The counter is not likely to roll unless catalogs keep on being
-      // opened/closed which is consider as an application mistake for the
-      // moment.
-      if (_M_catalog_counter == numeric_limits<catalog>::max())
-	return -1;
-
-      std::auto_ptr<Catalog_info> info(new Catalog_info(_M_catalog_counter++,
-							__domain, __l));
-      _M_infos.push_back(info.get());
-      return info.release()->_M_id;
-    }
-
-    void
-    _M_erase(catalog __c)
-    {
-      __gnu_cxx::__scoped_lock lock(_M_mutex);
-
-      vector<Catalog_info*>::iterator __res =
-	lower_bound(_M_infos.begin(), _M_infos.end(), __c, _Comp());
-      if (__res == _M_infos.end() || (*__res)->_M_id != __c)
-	return;
-
-      delete *__res;
-      _M_infos.erase(__res);
-
-      // Just in case closed catalog was the last open.
-      if (__c == _M_catalog_counter - 1)
-	--_M_catalog_counter;
-    }
-
-    const Catalog_info*
-    _M_get(catalog __c) const
-    {
-      __gnu_cxx::__scoped_lock lock(_M_mutex);
-
-      vector<Catalog_info*>::const_iterator __res =
-	lower_bound(_M_infos.begin(), _M_infos.end(), __c, _Comp());
-
-      if (__res != _M_infos.end() && (*__res)->_M_id == __c)
-	return *__res;
-
-      return 0;
-    }
-
-  private:
-    struct _Comp
-    {
-      bool operator()(catalog __cat, const Catalog_info* __info) const
-      { return __cat < __info->_M_id; }
-
-      bool operator()(const Catalog_info* __info, catalog __cat) const
-      { return __info->_M_id < __cat; }
-    };
-
-    mutable __gnu_cxx::__mutex _M_mutex;
-    catalog _M_catalog_counter;
-    std::vector<Catalog_info*> _M_infos;
-  };
-
-  Catalogs&
-  get_catalogs()
-  {
-    static Catalogs __catalogs;
-    return __catalogs;
-  }
-
   const char*
   get_glibc_msg(__c_locale __locale_messages __attribute__((unused)),
 		const char* __name_messages __attribute__((unused)),
@@ -180,7 +78,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       bind_textdomain_codeset(__s.c_str(),
 	  __nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
-      return get_catalogs()._M_add(__s, __l);
+      return get_catalogs()._M_add(__s.c_str(), __l);
     }
 
   template<>
@@ -202,7 +100,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return __dfault;
 
       return get_glibc_msg(_M_c_locale_messages, _M_name_messages,
-			   __cat_info->_M_domain.c_str(),
+			   __cat_info->_M_domain,
 			   __dfault.c_str());
     }
 
@@ -218,7 +116,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       bind_textdomain_codeset(__s.c_str(),
 	  __nl_langinfo_l(CODESET, __codecvt._M_c_locale_codecvt));
 
-      return get_catalogs()._M_add(__s, __l);
+      return get_catalogs()._M_add(__s.c_str(), __l);
     }
 
   template<>
@@ -248,7 +146,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __builtin_memset(&__state, 0, sizeof(mbstate_t));
       {
 	const wchar_t* __wdfault_next;
-	size_t __mb_size = __wdfault.size() * __conv.max_length();;
+	size_t __mb_size = __wdfault.size() * __conv.max_length();
 	char* __dfault =
 	  static_cast<char*>(__builtin_alloca(sizeof(char) * (__mb_size + 1)));
 	char* __dfault_next;
@@ -260,7 +158,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	// Make sure string passed to dgettext is \0 terminated.
 	*__dfault_next = '\0';
 	__translation = get_glibc_msg(_M_c_locale_messages, _M_name_messages,
-				      __cat_info->_M_domain.c_str(), __dfault);
+				      __cat_info->_M_domain, __dfault);
 
 	// If we end up getting default value back we can simply return original
 	// default value.
diff --git a/libstdc++-v3/src/c++98/Makefile.am b/libstdc++-v3/src/c++98/Makefile.am
index a5b68a1..c5a8866 100644
--- a/libstdc++-v3/src/c++98/Makefile.am
+++ b/libstdc++-v3/src/c++98/Makefile.am
@@ -155,6 +155,12 @@ vpath % $(top_srcdir)/src/c++98
 
 libc__98convenience_la_SOURCES = $(sources)
 
+# Use special rules to compile with -fimplicit-templates.
+c++locale.lo: c++locale.cc
+	$(LTCXXCOMPILE) -fimplicit-templates -c $<
+c++locale.o: c++locale.cc
+	$(CXXCOMPILE) -fimplicit-templates -c $<
+
 if ENABLE_DUAL_ABI
 GLIBCXX_ABI_FLAGS = -D_GLIBCXX_USE_CXX11_ABI=@glibcxx_cxx98_abi@
 # Use special rules to compile with the non-default string ABI.
diff --git a/libstdc++-v3/src/c++98/Makefile.in b/libstdc++-v3/src/c++98/Makefile.in
index b1a1b49..3c3bbbd 100644
--- a/libstdc++-v3/src/c++98/Makefile.in
+++ b/libstdc++-v3/src/c++98/Makefile.in
@@ -776,6 +776,12 @@ basic_file.cc: ${glibcxx_srcdir}/$(BASIC_FILE_CC)
 	$(LN_S) ${glibcxx_srcdir}/$(BASIC_FILE_CC) ./$@ || true
 
 vpath % $(top_srcdir)/src/c++98
+
+# Use special rules to compile with -fimplicit-templates.
+c++locale.lo: c++locale.cc
+	$(LTCXXCOMPILE) -fimplicit-templates -c $<
+c++locale.o: c++locale.cc
+	$(CXXCOMPILE) -fimplicit-templates -c $<
 # Use special rules to compile with the non-default string ABI.
 @ENABLE_DUAL_ABI_TRUE@collate_members_cow.lo: collate_members_cow.cc
 @ENABLE_DUAL_ABI_TRUE@	$(LTCXXCOMPILE) $(GLIBCXX_ABI_FLAGS) -fimplicit-templates -c $<