[patch] c++/65046 Use abi-tag to fix building with -Werror=abi-tag

[patch] c++/65046 Use abi-tag to fix building with -Werror=abi-tag

[Jonathan Wakely]

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

This adds the abi_tag attribute in a few places it's missing, so that
-Werror=abi-tag builds will work after Jason's upcoming patches to
make functions inherit the tag from their return type.

For the __sso_string type I disable the warning, because the fact it
uses std::__cxx11::string internally is not visible to users.

There are still some missing tags in the Debug Mode string and list
which I'll fix asap.

Preparing this patch reminded me that we currently have two copies of
the Catalog_info and Catalogs code in the unnamed namespace in
config/locale/gnu/messages_members.cc, one using the old string and
one using the new. We should really alter the code to not use
std::string so that the catalogs can be shared by both versions of the
messages facets.

Tested x86_64-linux, as a standard build, and also with Jason's WIP
patches and --enable-cxx-flags=-Werror=abi-tag.

Committed to trunk.


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

commit b6a949d6488a8ee72f182e7d859c830b45bf71a0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Mar 17 16:45:32 2015 +0000

    	PR c++/65046
    	* config/locale/gnu/messages_members.cc (Catalog_info, Catalogs,
    	get_catalogs): Add abi-tag.
    	* include/ext/codecvt_specializations.h (encoding_state,
    	encoding_char_traits): Likewise.
    	* src/c++11/cxx11-ios_failure.cc (io_error_category): Likewise.
    	* src/c++11/cxx11-shim_facets.cc (__any_string::operator basic_string,
    	numpunct_shim, collate_shim, time_get_shim, moneypunct_shim,
    	money_get_shim, money_put_shim, messages_shim): Likewise.
    	* src/c++11/future.cc (future_error_category::message): Likewise.
    	* src/c++11/system_error.cc (generic_error_category::message,
    	system_error_category::message): Likewise.
    	(__sso_string): Disable -Wabi-tag warnings.

diff --git a/libstdc++-v3/config/locale/gnu/messages_members.cc b/libstdc++-v3/config/locale/gnu/messages_members.cc
index 2e6122d..c90499e 100644
--- a/libstdc++-v3/config/locale/gnu/messages_members.cc
+++ b/libstdc++-v3/config/locale/gnu/messages_members.cc
@@ -46,8 +46,8 @@ namespace
 
   typedef messages_base::catalog catalog;
 
-  struct Catalog_info
-    {
+  struct _GLIBCXX_DEFAULT_ABI_TAG Catalog_info
+  {
     Catalog_info(catalog __id, const string& __domain, locale __loc)
       : _M_id(__id), _M_domain(__domain), _M_locale(__loc)
     { }
@@ -57,7 +57,7 @@ namespace
     locale _M_locale;
   };
 
-  class Catalogs
+  class _GLIBCXX_DEFAULT_ABI_TAG Catalogs
   {
   public:
     Catalogs() : _M_catalog_counter(0) { }
@@ -133,6 +133,7 @@ namespace
     std::vector<Catalog_info*> _M_infos;
   };
 
+  _GLIBCXX_DEFAULT_ABI_TAG
   Catalogs&
   get_catalogs()
   {
diff --git a/libstdc++-v3/include/ext/codecvt_specializations.h b/libstdc++-v3/include/ext/codecvt_specializations.h
index a24adfc..d9f6630 100644
--- a/libstdc++-v3/include/ext/codecvt_specializations.h
+++ b/libstdc++-v3/include/ext/codecvt_specializations.h
@@ -47,7 +47,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // This includes conversions and comparisons between various character
   // sets.  This object encapsulates data that may need to be shared between
   // char_traits, codecvt and ctype.
-  class encoding_state
+  class _GLIBCXX_DEFAULT_ABI_TAG encoding_state
   {
   public:
     // Types: 
@@ -207,7 +207,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // associated fpos<encoding_state> for the position type, all other
   // bits equivalent to the required char_traits instantiations.
   template<typename _CharT>
-    struct encoding_char_traits : public std::char_traits<_CharT>
+    struct _GLIBCXX_DEFAULT_ABI_TAG encoding_char_traits
+    : public std::char_traits<_CharT>
     {
       typedef encoding_state				state_type;
       typedef typename std::fpos<state_type>		pos_type;
diff --git a/libstdc++-v3/src/c++11/cxx11-ios_failure.cc b/libstdc++-v3/src/c++11/cxx11-ios_failure.cc
index e1c8d4e..b0a7c46 100644
--- a/libstdc++-v3/src/c++11/cxx11-ios_failure.cc
+++ b/libstdc++-v3/src/c++11/cxx11-ios_failure.cc
@@ -41,6 +41,7 @@ namespace
     name() const noexcept
     { return "iostream"; }
 
+    _GLIBCXX_DEFAULT_ABI_TAG
     virtual std::string message(int __ec) const
     {
       std::string __msg;
diff --git a/libstdc++-v3/src/c++11/cxx11-shim_facets.cc b/libstdc++-v3/src/c++11/cxx11-shim_facets.cc
index 82bdf6f..a32b9f0 100644
--- a/libstdc++-v3/src/c++11/cxx11-shim_facets.cc
+++ b/libstdc++-v3/src/c++11/cxx11-shim_facets.cc
@@ -147,6 +147,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     // The returned object will match the caller's string ABI, even when the
     // stored string doesn't.
     template<typename C>
+      _GLIBCXX_DEFAULT_ABI_TAG
       operator basic_string<C>() const
       {
 	if (!_M_dtor)
@@ -226,7 +227,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   namespace // unnamed
   {
     template<typename _CharT>
-      struct numpunct_shim : std::numpunct<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG numpunct_shim
+      : std::numpunct<_CharT>, facet::__shim
       {
 	typedef typename numpunct<_CharT>::__cache_type __cache_type;
 
@@ -250,7 +252,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT>
-      struct collate_shim : std::collate<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG collate_shim
+      : std::collate<_CharT>, facet::__shim
       {
 	typedef basic_string<_CharT>	string_type;
 
@@ -275,7 +278,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT>
-      struct time_get_shim : std::time_get<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG time_get_shim
+      : std::time_get<_CharT>, facet::__shim
       {
 	typedef typename std::time_get<_CharT>::iter_type iter_type;
 	typedef typename std::time_get<_CharT>::char_type char_type;
@@ -329,7 +333,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT, bool _Intl>
-      struct moneypunct_shim : std::moneypunct<_CharT, _Intl>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG moneypunct_shim
+      : std::moneypunct<_CharT, _Intl>, facet::__shim
       {
 	typedef typename moneypunct<_CharT, _Intl>::__cache_type __cache_type;
 
@@ -356,7 +361,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT>
-      struct money_get_shim : std::money_get<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG money_get_shim
+      : std::money_get<_CharT>, facet::__shim
       {
 	typedef typename std::money_get<_CharT>::iter_type iter_type;
 	typedef typename std::money_get<_CharT>::char_type char_type;
@@ -397,7 +403,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT>
-      struct money_put_shim : std::money_put<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG money_put_shim
+      : std::money_put<_CharT>, facet::__shim
       {
 	typedef typename std::money_put<_CharT>::iter_type iter_type;
 	typedef typename std::money_put<_CharT>::char_type char_type;
@@ -426,7 +433,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       };
 
     template<typename _CharT>
-      struct messages_shim : std::messages<_CharT>, facet::__shim
+      struct _GLIBCXX_DEFAULT_ABI_TAG messages_shim
+      : std::messages<_CharT>, facet::__shim
       {
 	typedef messages_base::catalog  catalog;
 	typedef basic_string<_CharT>	string_type;
diff --git a/libstdc++-v3/src/c++11/future.cc b/libstdc++-v3/src/c++11/future.cc
index 3cf503b..21dbd8a 100644
--- a/libstdc++-v3/src/c++11/future.cc
+++ b/libstdc++-v3/src/c++11/future.cc
@@ -32,6 +32,7 @@ namespace
     name() const noexcept
     { return "future"; }
 
+    _GLIBCXX_DEFAULT_ABI_TAG
     virtual std::string message(int __ec) const
     {
       std::string __msg;
diff --git a/libstdc++-v3/src/c++11/system_error.cc b/libstdc++-v3/src/c++11/system_error.cc
index 71f5c8b..b7ac1f8 100644
--- a/libstdc++-v3/src/c++11/system_error.cc
+++ b/libstdc++-v3/src/c++11/system_error.cc
@@ -41,6 +41,7 @@ namespace
     name() const noexcept
     { return "generic"; }
 
+    _GLIBCXX_DEFAULT_ABI_TAG
     virtual string 
     message(int i) const
     {
@@ -56,6 +57,7 @@ namespace
     name() const noexcept
     { return "system"; }
 
+    _GLIBCXX_DEFAULT_ABI_TAG
     virtual string
     message(int i) const
     {
@@ -111,6 +113,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 
 #if _GLIBCXX_USE_DUAL_ABI
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wabi-tag"
   // Redefine __sso_string so that we can define and export its members
   // in terms of the SSO std::string.
   struct __sso_string
@@ -137,6 +141,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     __sso_string(__sso_string&&) noexcept;
     __sso_string& operator=(__sso_string&&) noexcept;
   };
+#pragma GCC diagnostic pop
 
   __sso_string::__sso_string() : _M_str() { }
 

Make messages_members.cc Catalog_info and Catalogs ABI agnostic

[François Dumont]

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

On 18/03/2015 19:16, Jonathan Wakely wrote:
>
> Preparing this patch reminded me that we currently have two copies of
> the Catalog_info and Catalogs code in the unnamed namespace in
> config/locale/gnu/messages_members.cc, one using the old string and
> one using the new. We should really alter the code to not use
> std::string so that the catalogs can be shared by both versions of the
> messages facets.
>
Hello

     Do you mean like the attached patch ? Or do I need to isolate 
get_catalogs function in a dedicated source file that won't be built twice ?

     Tested under Linux x86_64 but uniqueness of catalogs have not been 
checked.

François


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

diff --git a/libstdc++-v3/config/locale/gnu/messages_members.cc b/libstdc++-v3/config/locale/gnu/messages_members.cc
index c90499e..c34d846 100644
--- a/libstdc++-v3/config/locale/gnu/messages_members.cc
+++ b/libstdc++-v3/config/locale/gnu/messages_members.cc
@@ -46,18 +46,21 @@ namespace
 
   typedef messages_base::catalog catalog;
 
-  struct _GLIBCXX_DEFAULT_ABI_TAG Catalog_info
+  struct Catalog_info
   {
-    Catalog_info(catalog __id, const string& __domain, locale __loc)
-      : _M_id(__id), _M_domain(__domain), _M_locale(__loc)
+    Catalog_info(catalog __id, const char* __domain, locale __loc)
+      : _M_id(__id), _M_domain(strdup(__domain)), _M_locale(__loc)
     { }
 
+    ~Catalog_info()
+    { free(_M_domain); }
+
     catalog _M_id;
-    string _M_domain;
+    char* _M_domain;
     locale _M_locale;
   };
 
-  class _GLIBCXX_DEFAULT_ABI_TAG Catalogs
+  class Catalogs
   {
   public:
     Catalogs() : _M_catalog_counter(0) { }
@@ -70,7 +73,7 @@ namespace
     }
 
     catalog
-    _M_add(const string& __domain, locale __l)
+    _M_add(const char* __domain, locale __l)
     {
       __gnu_cxx::__scoped_lock lock(_M_mutex);
 
@@ -82,6 +85,10 @@ namespace
 
       std::auto_ptr<Catalog_info> info(new Catalog_info(_M_catalog_counter++,
 							__domain, __l));
+
+      if (!info->_M_domain)
+	return -1;
+
       _M_infos.push_back(info.get());
       return info.release()->_M_id;
     }
@@ -133,7 +140,6 @@ namespace
     std::vector<Catalog_info*> _M_infos;
   };
 
-  _GLIBCXX_DEFAULT_ABI_TAG
   Catalogs&
   get_catalogs()
   {
@@ -181,7 +187,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<>
@@ -203,7 +209,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());
     }
 
@@ -219,7 +225,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<>
@@ -261,7 +267,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.

Re: Make messages_members.cc Catalog_info and Catalogs ABI agnostic

[Jonathan Wakely]

On 19/03/15 21:50 +0100, François Dumont wrote:
>On 18/03/2015 19:16, Jonathan Wakely wrote:
>>
>>Preparing this patch reminded me that we currently have two copies of
>>the Catalog_info and Catalogs code in the unnamed namespace in
>>config/locale/gnu/messages_members.cc, one using the old string and
>>one using the new. We should really alter the code to not use
>>std::string so that the catalogs can be shared by both versions of the
>>messages facets.
>>
>Hello
>
>    Do you mean like the attached patch ? Or do I need to isolate 
>get_catalogs function in a dedicated source file that won't be built 
>twice ?

That's a partial fix, but the types and functions are still in an
anonymous namespace, so will still be duplicated.

To do it correctly they can't be in an anon namespace and the names
must be uglified.

Re: [patch] c++/65046 Use abi-tag to fix building with -Werror=abi-tag

[Jonathan Wakely]

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

On 18/03/15 18:16 +0000, Jonathan Wakely wrote:
>This adds the abi_tag attribute in a few places it's missing, so that
>-Werror=abi-tag builds will work after Jason's upcoming patches to
>make functions inherit the tag from their return type.

Jason suggested using an inline namespace instead of tagging types, so
this adds __gnu_cxx::__cxx11, similar to the existing std::__cxx11.

Tested x86_64-linux, committed to trunk.


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

commit de4ceb0c3bc9cc833c044683c9d94ccb60812359
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Mar 20 10:49:18 2015 +0000

    	* include/bits/c++config (__gnu_cxx::__cxx11): Define new namespace.
    	* include/ext/codecvt_specializations.h (encoding_state,
    	encoding_char_traits): Remove abi-tag and use inline namespace.
    	* testsuite/ext/profile/mutex_extensions_neg.cc: Adjust dg-error line.

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index eebe34c..ae3065f 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -217,6 +217,10 @@ namespace std
 {
   inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
 }
+namespace __gnu_cxx
+{
+  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+}
 # define _GLIBCXX_NAMESPACE_CXX11 __cxx11::
 # define _GLIBCXX_BEGIN_NAMESPACE_CXX11 namespace __cxx11 {
 # define _GLIBCXX_END_NAMESPACE_CXX11 }
diff --git a/libstdc++-v3/include/ext/codecvt_specializations.h b/libstdc++-v3/include/ext/codecvt_specializations.h
index d9f6630..34e90bd 100644
--- a/libstdc++-v3/include/ext/codecvt_specializations.h
+++ b/libstdc++-v3/include/ext/codecvt_specializations.h
@@ -41,13 +41,14 @@
 
 namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 {
+_GLIBCXX_BEGIN_NAMESPACE_CXX11
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /// Extension to use iconv for dealing with character encodings.
   // This includes conversions and comparisons between various character
   // sets.  This object encapsulates data that may need to be shared between
   // char_traits, codecvt and ctype.
-  class _GLIBCXX_DEFAULT_ABI_TAG encoding_state
+  class encoding_state
   {
   public:
     // Types: 
@@ -207,7 +208,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // associated fpos<encoding_state> for the position type, all other
   // bits equivalent to the required char_traits instantiations.
   template<typename _CharT>
-    struct _GLIBCXX_DEFAULT_ABI_TAG encoding_char_traits
+    struct encoding_char_traits
     : public std::char_traits<_CharT>
     {
       typedef encoding_state				state_type;
@@ -215,6 +216,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     };
 
 _GLIBCXX_END_NAMESPACE_VERSION
+_GLIBCXX_END_NAMESPACE_CXX11
 } // namespace
 
 
diff --git a/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc b/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
index d7a0c56..dd19f14 100644
--- a/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
+++ b/libstdc++-v3/testsuite/ext/profile/mutex_extensions_neg.cc
@@ -25,4 +25,4 @@
 
 #include <vector>
 
-// { dg-error "multiple inlined namespaces" "" { target *-*-* } 318 }
+// { dg-error "multiple inlined namespaces" "" { target *-*-* } 322 }