From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102560 invoked by alias); 29 Sep 2015 19:49:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 102541 invoked by uid 89); 29 Sep 2015 19:49:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wi0-f178.google.com Received: from mail-wi0-f178.google.com (HELO mail-wi0-f178.google.com) (209.85.212.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 29 Sep 2015 19:49:25 +0000 Received: by wicgb1 with SMTP id gb1so164735547wic.1; Tue, 29 Sep 2015 12:49:22 -0700 (PDT) X-Received: by 10.194.52.6 with SMTP id p6mr34896652wjo.119.1443556162712; Tue, 29 Sep 2015 12:49:22 -0700 (PDT) Received: from [192.168.0.23] ([82.237.250.248]) by smtp.googlemail.com with ESMTPSA id xa5sm18858026wjc.20.2015.09.29.12.49.14 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Sep 2015 12:49:21 -0700 (PDT) Subject: Re: Elimitate duplication of get_catalogs in different abi To: Jonathan Wakely References: <55B694F5.3070606@gmail.com> <55BA81B1.9030206@gmail.com> <20150805205744.GC13355@redhat.com> <55D79415.90600@gmail.com> <55EB586A.8080405@gmail.com> <5602FD59.2020805@gmail.com> <20150925150833.GH12094@redhat.com> <20150925151052.GI12094@redhat.com> <20150925155855.GJ12094@redhat.com> Cc: "libstdc++@gcc.gnu.org" , gcc-patches From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= X-Enigmail-Draft-Status: N1110 Message-ID: <560AEB37.9070007@gmail.com> Date: Tue, 29 Sep 2015 20:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150925155855.GJ12094@redhat.com> Content-Type: multipart/mixed; boundary="------------040107010808020204080201" X-SW-Source: 2015-09/txt/msg02254.txt.bz2 This is a multi-part message in MIME format. --------------040107010808020204080201 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Content-length: 3120 On 25/09/2015 17:58, Jonathan Wakely wrote: > On 25/09/15 16:10 +0100, Jonathan Wakely wrote: >> On 25/09/15 16:08 +0100, Jonathan Wakely wrote: >>> On 23/09/15 21:28 +0200, François Dumont wrote: >>>> On 05/09/2015 23:02, François Dumont wrote: >>>>> On 22/08/2015 14:24, Daniel Krügler wrote: >>>>>> 2015-08-21 23:11 GMT+02:00 François Dumont : >>>>>>> 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 >>>>> >>>> >>>> What about this patch ? >>>> >>>> I am still uncomfortable in exposing those implementation details >>>> in the >>>> versionned symbols but I don't know how to do otherwise. Do you >>>> want me >>>> to push this code in std::__detail namespace ? >>> >>> I think because the types are only used internally in the library we >>> don't need to export them. The other code inside the shared library >>> can refer to those symbols without them being exported. >>> >>> That way users can't see their names (because they're not in any >>> public headers) and can't use the symbols (because they're not >>> exported) so they're pure internal implementation details. >>> >>> I tested it briefly and it seems to work, so if you can confirm it >>> still works then the patch is OK without the changes to gnu.ver >> >> Oh, the problem is that the symbols are matched by patterns in the >> _GLIBCXX_3.4 version, so get exported with that version instead. Gah. >> >> In that case your patch would not have worked on Solaris anyway, as >> the SOlaris linker gives an error if a symbol matches patterns in more >> than one symbol version. >> >> Let me try to adjust the gnu.ver script to make this work ... > > This should do it ... > Indeed, I just rerun all tests with success. I am re-attaching the patch. 2015-09-30 François Dumont Jonathan Wakely * config/locale/gnu/messages_members.cc (Catalog_info, Catalogs): Move... * config/locale/gnu/c++locale_internal.h: ...here in std namespace. * config/locale/gnu/c_locale.cc: Move implementation of latter here. * config/abi/pre/gnu.ver: Adjust. Ok to commit ? François --------------040107010808020204080201 Content-Type: text/x-patch; name="messages_catalogs.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="messages_catalogs.patch" Content-length: 12133 diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index d42cd37..c761052 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -24,7 +24,7 @@ GLIBCXX_3.4 { # Names inside the 'extern' block are demangled names. extern "C++" { - std::[A-Z]*; + std::[ABD-Z]*; std::a[a-c]*; std::ad[a-n]*; std::ad[p-z]*; @@ -106,7 +106,7 @@ GLIBCXX_3.4 { # std::istringstream*; std::istrstream*; std::i[t-z]*; - std::[A-Zj-k]*; + std::[j-k]*; # std::length_error::l*; # std::length_error::~l*; std::locale::[A-Za-e]*; @@ -132,9 +132,8 @@ GLIBCXX_3.4 { # std::logic_error::l*; std::logic_error::what*; # std::logic_error::~l*; -# std::[A-Zm-r]*; -# std::[A-Zm]*; - std::[A-Z]*; +# std::[m-r]*; +# std::[m]*; std::messages[^_]*; # std::messages_byname*; std::money_*; @@ -175,11 +174,13 @@ GLIBCXX_3.4 { # std::t[i-n]*; std::tr1::h[^a]*; std::t[s-z]*; -# std::[A-Zu-z]*; +# std::[u-z]*; # std::underflow_error::u*; # std::underflow_error::~u*; std::unexpected*; - std::[A-Zv-z]*; + std::valarray*; + # std::vector* + std::[w-z]*; std::_List_node_base::hook*; std::_List_node_base::swap*; std::_List_node_base::unhook*; 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 #include +#include +#include // ::strdup + +#include + #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 _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..0d6d204 100644 --- a/libstdc++-v3/config/locale/gnu/c_locale.cc +++ b/libstdc++-v3/config/locale/gnu/c_locale.cc @@ -31,9 +31,12 @@ #include #include #include +#include #include #include +#include + 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::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::max()) + return -1; + + auto_ptr 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::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::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 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 #include -#include -#include -#include #include // std::free #include // ::strdup -#include -#include - 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::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::max()) - return -1; - - std::auto_ptr 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::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::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 _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(__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 $< --------------040107010808020204080201--