| Message ID | 1387466908.2455.355.camel@yam-132-YW-E178-FTW |
|---|---|
| State | New |
| Headers | show |
Ping. The patch in question is here: http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01688.html On Thu, 2013-12-19 at 16:28 +0100, Oleg Endo wrote: > On Thu, 2013-12-19 at 01:19 +0000, Jonathan Wakely wrote: > > On 19 December 2013 00:10, Oleg Endo wrote: > > > Hello, > > > > > > When writing code such as > > > ... > > > throw std::logic_error ("cold coffee"); > > > ... > > > currently the construction of std::string happens in the code that > > > throws the exception, which results in code bloat. Implementing the > > > const char* constructors as defined by C++11 fixes the issue. > > > I'm not sure whether the #if __cplusplus >= 201103L checks are required. > > > C++98 code could also benefit from the overloads. > > > > I think there was some good reason we haven't added these yet, but I > > can't remember it. > > > > > Tested with 'make all' and 'make install', writing a hello world and > > > checking the asm output. > > > > For all patches we need to know that the libstdc++ testsuite passes too. > > Right, I should have done that in the first place. The patch was > broken. Sorry for the noise. > > Files in libstdc++-v3/src/c++98/ seem to be never compiled with C++11. > Thus I can think of two options: > 1) Add const char* ctors for C++98 and C++11. > 2) Add #ifdef'ed declarations to libstdc++-v3/include/std/stdexcept and > add a new file libstdc++-v3/src/c++11/stdexcept.cc with the > implementations. > > The attached patch does 1). > > Tested with 'make all' and 'make check-target-libstdc++-v3' on > i686-pc-linux-gnu, configured with: > ../gcc-trunk2/configure --prefix=/<...> --enable-languages=c,c++ > > === libstdc++ Summary === > > # of expected passes 5142 > # of unexpected failures 2 > # of expected failures 34 > # of unsupported tests 476 > > Cheers, > Oleg > > libstdc++-v3/ChangeLog: > * include/std/stdexcept (logic_error, domain_error, > invalid_argument, length_error, out_of_range, runtime_error, > range_error, overflow_error, underflow_error): Declare const > char* constructors. > * src/c++98/stdexcept.cc (logic_error, domain_error, > invalid_argument, length_error, out_of_range, runtime_error, > range_error, overflow_error, underflow_error): Implement them. > * config/abi/pre/gnu.ver ( _ZNSt11logic_errorC[12]EPKc, > _ZNSt12domain_errorC[12]EPKc, _ZNSt16invalid_argumentC[12]EPKc, > _ZNSt12length_errorC[12]EPKc, _ZNSt12out_of_rangeC[12]EPKc, > _ZNSt13runtime_errorC[12]EPKc, _ZNSt11range_errorC[12]EPKc, > _ZNSt14overflow_errorC[12]EPKc, _ZNSt15underflow_errorC[12]EPKc): > Add new exports. > * doc/xml/manual/status_cxx2011.xml: Update.
On 26/01/14 15:15 +0100, Oleg Endo wrote: >> Files in libstdc++-v3/src/c++98/ seem to be never compiled with C++11. >> Thus I can think of two options: >> 1) Add const char* ctors for C++98 and C++11. >> 2) Add #ifdef'ed declarations to libstdc++-v3/include/std/stdexcept and >> add a new file libstdc++-v3/src/c++11/stdexcept.cc with the >> implementations. 3) Move stdexcept.cc from src/c++98 to src/c++11 4) Define the functions inline using forwarding constructors, which means we don't need new exports. >> The attached patch does 1). I don't think we want the constructors in C++03 mode though, it could break some valid programs e.g. a custom string type with implicit conversion to const char* and std::string can be passed to an exception constructor in C++03, but adding the overloads would make it ambiguous. I think I prefer option 4, it avoids adding new exports during Stage 3 (although the patch was initially posted during Stage 1, it is now quite late to add new exports, which is not your fault but still a concern.)
On Wed, 2014-01-29 at 15:21 +0000, Jonathan Wakely wrote: > On 26/01/14 15:15 +0100, Oleg Endo wrote: > >> Files in libstdc++-v3/src/c++98/ seem to be never compiled with C++11. > >> Thus I can think of two options: > >> 1) Add const char* ctors for C++98 and C++11. > >> 2) Add #ifdef'ed declarations to libstdc++-v3/include/std/stdexcept and > >> add a new file libstdc++-v3/src/c++11/stdexcept.cc with the > >> implementations. > > 3) Move stdexcept.cc from src/c++98 to src/c++11 > 4) Define the functions inline using forwarding constructors, which > means we don't need new exports. > > >> The attached patch does 1). > > I don't think we want the constructors in C++03 mode though, it could > break some valid programs e.g. a custom string type with implicit > conversion to const char* and std::string can be passed to an exception > constructor in C++03, but adding the overloads would make it > ambiguous. Good point. > > I think I prefer option 4, it avoids adding new exports during Stage 3 > (although the patch was initially posted during Stage 1, it is now > quite late to add new exports, which is not your fault but still a > concern.) My original intention was to eliminate code bloat when doing something like throw std::logic_error ("cold coffee"); If the const char* overloads are inlined it will emit code to construct an std::string from const char* in user code where the exception is being constructed over and over again. The idea was to move that code into the std library. BTW the original patch was posted during Stage 3 (19.12.2013). I don't mind waiting until Stage 1 if adding exports now is a problem. Cheers, Oleg
On 29 January 2014 21:17, Oleg Endo wrote: > My original intention was to eliminate code bloat when doing something > like throw std::logic_error ("cold coffee"); > If the const char* overloads are inlined it will emit code to construct > an std::string from const char* in user code where the exception is > being constructed over and over again. The idea was to move that code > into the std library. That's exactly what happens today with the constructors that only take a std::string, so it wouldn't be any worse than what we have now, would it? > BTW the original patch was posted during Stage 3 (19.12.2013). I don't > mind waiting until Stage 1 if adding exports now is a problem. OK, let's wait and decide how we want to do it properly in stage 1. (If we're going to make various changes that impact the ABI during the next stage 1 we might even want to consider changing the std::exception base class to store something like a std::shared_ptr<std::string> so that copying an exception object will never throw an exception, which is something I've been thinking about recently.)
On Wed, 2014-01-29 at 21:38 +0000, Jonathan Wakely wrote: > On 29 January 2014 21:17, Oleg Endo wrote: > > My original intention was to eliminate code bloat when doing something > > like throw std::logic_error ("cold coffee"); > > If the const char* overloads are inlined it will emit code to construct > > an std::string from const char* in user code where the exception is > > being constructed over and over again. The idea was to move that code > > into the std library. > > That's exactly what happens today with the constructors that only take > a std::string, so it wouldn't be any worse than what we have now, > would it? Sorry, I'm not sure I understand your question. Maybe you meant "any better than what we have"? Anyway, I've attached two outputs that show what I mean. The version with the const char* ctor overloads implemented in the library is significantly shorter in the throwing path (5x function call vs. 7x function call + other inlined std::string ctor code). > > > BTW the original patch was posted during Stage 3 (19.12.2013). I don't > > mind waiting until Stage 1 if adding exports now is a problem. > > OK, let's wait and decide how we want to do it properly in stage 1. Sure. Actually I've missed some of the other exception types in system_error, which should be added, too. That would eliminate the TODO: Add const char* ctors to all exceptions. I'd also propose moving the system_error ctor implementations into the library as well, for the same reasons as above. > > (If we're going to make various changes that impact the ABI during the > next stage 1 we might even want to consider changing the > std::exception base class to store something like a > std::shared_ptr<std::string> so that copying an exception object will > never throw an exception, which is something I've been thinking about > recently.) Wouldn't using std::shared_ptr<std::string> introduce an additional heap allocation when creating the exception, though? How about storing the exception message in a plain zero terminated string instead? This would require only one heap allocation if the string data is prefixed with a refcount variable. Basically, std::make_shared re-invented because it doesn't work with arrays. Or maybe implement N3640 + N3641 first (even if for library internal use only as a start)... Cheers, Oleg
Jonathan, now that we're in stage 1 again, I'd like to revive the issue below. Do you have any particular plans? How should we proceed? Cheers, Oleg On Wed, 2014-01-29 at 23:45 +0100, Oleg Endo wrote: > On Wed, 2014-01-29 at 21:38 +0000, Jonathan Wakely wrote: > > On 29 January 2014 21:17, Oleg Endo wrote: > > > My original intention was to eliminate code bloat when doing something > > > like throw std::logic_error ("cold coffee"); > > > If the const char* overloads are inlined it will emit code to construct > > > an std::string from const char* in user code where the exception is > > > being constructed over and over again. The idea was to move that code > > > into the std library. > > > > That's exactly what happens today with the constructors that only take > > a std::string, so it wouldn't be any worse than what we have now, > > would it? > > Sorry, I'm not sure I understand your question. Maybe you meant "any > better than what we have"? Anyway, I've attached two outputs that show > what I mean. The version with the const char* ctor overloads > implemented in the library is significantly shorter in the throwing path > (5x function call vs. 7x function call + other inlined std::string ctor > code). > > > > > > BTW the original patch was posted during Stage 3 (19.12.2013). I don't > > > mind waiting until Stage 1 if adding exports now is a problem. > > > > OK, let's wait and decide how we want to do it properly in stage 1. > > Sure. Actually I've missed some of the other exception types in > system_error, which should be added, too. That would eliminate the > TODO: Add const char* ctors to all exceptions. > > I'd also propose moving the system_error ctor implementations into the > library as well, for the same reasons as above. > > > > > (If we're going to make various changes that impact the ABI during the > > next stage 1 we might even want to consider changing the > > std::exception base class to store something like a > > std::shared_ptr<std::string> so that copying an exception object will > > never throw an exception, which is something I've been thinking about > > recently.) > > Wouldn't using std::shared_ptr<std::string> introduce an additional heap > allocation when creating the exception, though? > How about storing the exception message in a plain zero terminated > string instead? This would require only one heap allocation if the > string data is prefixed with a refcount variable. Basically, > std::make_shared re-invented because it doesn't work with arrays. > Or maybe implement N3640 + N3641 first (even if for library internal use > only as a start)... > > Cheers, > Oleg
On 1 May 2014 17:18, Oleg Endo wrote: > Jonathan, > > now that we're in stage 1 again, I'd like to revive the issue below. Do > you have any particular plans? How should we proceed? Hi Oleg, sorry for letting the thread die in January. We will definitely want to deal with the missing constructors in <stdexcept> during stage 1, but I'm not sure exactly when :-) I'll come back to this next week - ping me if you don't hear from me again!
On 02 May 2014, at 17:57, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On 1 May 2014 17:18, Oleg Endo wrote: >> Jonathan, >> >> now that we're in stage 1 again, I'd like to revive the issue below. Do >> you have any particular plans? How should we proceed? > > Hi Oleg, sorry for letting the thread die in January. > > We will definitely want to deal with the missing constructors in > <stdexcept> during stage 1, but I'm not sure exactly when :-) > > I'll come back to this next week - ping me if you don't hear from me again! And here I am, pinging... :) If it helps, I could brush up the patch that I submitted here: http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01688.html The thing you've mentioned regarding exception message string storage in http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01925.html is a different issue I think. Cheers, Oleg
Index: libstdc++-v3/config/abi/pre/gnu.ver =================================================================== --- libstdc++-v3/config/abi/pre/gnu.ver (revision 206101) +++ libstdc++-v3/config/abi/pre/gnu.ver (working copy) @@ -1371,6 +1371,33 @@ # std::regex_error::regex_error(std::regex_constants::error_type) _ZNSt11regex_errorC[01]ENSt15regex_constants10error_typeE; + # std::logic_error::logic_error(const char*) + _ZNSt11logic_errorC[12]EPKc; + + # std::domain_error::domain_error(const char*) + _ZNSt12domain_errorC[12]EPKc; + + # std::invalid_argument::invalid_argument(const char*) + _ZNSt16invalid_argumentC[12]EPKc; + + # std::length_error::length_error(const char*) + _ZNSt12length_errorC[12]EPKc; + + # std::out_of_range::out_of_range(const char*) + _ZNSt12out_of_rangeC[12]EPKc; + + # std::runtime_error::runtime_error(const char*) + _ZNSt13runtime_errorC[12]EPKc; + + # std::range_error::range_error(const char*) + _ZNSt11range_errorC[12]EPKc; + + # std::overflow_error::overflow_error(const char*) + _ZNSt14overflow_errorC[12]EPKc; + + # std::underflow_error::underflow_error(const char*) + _ZNSt15underflow_errorC[12]EPKc; + } GLIBCXX_3.4.19; # Symbols in the support library (libsupc++) have their own tag. Index: libstdc++-v3/src/c++98/stdexcept.cc =================================================================== --- libstdc++-v3/src/c++98/stdexcept.cc (revision 206101) +++ libstdc++-v3/src/c++98/stdexcept.cc (working copy) @@ -36,6 +36,9 @@ logic_error::logic_error(const string& __arg) : exception(), _M_msg(__arg) { } + logic_error::logic_error(const char* __arg) + : exception(), _M_msg(__arg) { } + logic_error::~logic_error() _GLIBCXX_USE_NOEXCEPT { } const char* @@ -45,26 +48,41 @@ domain_error::domain_error(const string& __arg) : logic_error(__arg) { } + domain_error::domain_error(const char* __arg) + : logic_error(__arg) { } + domain_error::~domain_error() _GLIBCXX_USE_NOEXCEPT { } invalid_argument::invalid_argument(const string& __arg) : logic_error(__arg) { } + invalid_argument::invalid_argument(const char* __arg) + : logic_error(__arg) { } + invalid_argument::~invalid_argument() _GLIBCXX_USE_NOEXCEPT { } length_error::length_error(const string& __arg) : logic_error(__arg) { } + length_error::length_error(const char* __arg) + : logic_error(__arg) { } + length_error::~length_error() _GLIBCXX_USE_NOEXCEPT { } out_of_range::out_of_range(const string& __arg) : logic_error(__arg) { } + out_of_range::out_of_range(const char* __arg) + : logic_error(__arg) { } + out_of_range::~out_of_range() _GLIBCXX_USE_NOEXCEPT { } runtime_error::runtime_error(const string& __arg) : exception(), _M_msg(__arg) { } + runtime_error::runtime_error(const char* __arg) + : exception(), _M_msg(__arg) { } + runtime_error::~runtime_error() _GLIBCXX_USE_NOEXCEPT { } const char* @@ -74,16 +92,25 @@ range_error::range_error(const string& __arg) : runtime_error(__arg) { } + range_error::range_error(const char* __arg) + : runtime_error(__arg) { } + range_error::~range_error() _GLIBCXX_USE_NOEXCEPT { } overflow_error::overflow_error(const string& __arg) : runtime_error(__arg) { } + overflow_error::overflow_error(const char* __arg) + : runtime_error(__arg) { } + overflow_error::~overflow_error() _GLIBCXX_USE_NOEXCEPT { } underflow_error::underflow_error(const string& __arg) : runtime_error(__arg) { } + underflow_error::underflow_error(const char* __arg) + : runtime_error(__arg) { } + underflow_error::~underflow_error() _GLIBCXX_USE_NOEXCEPT { } _GLIBCXX_END_NAMESPACE_VERSION Index: libstdc++-v3/doc/xml/manual/status_cxx2011.xml =================================================================== --- libstdc++-v3/doc/xml/manual/status_cxx2011.xml (revision 206101) +++ libstdc++-v3/doc/xml/manual/status_cxx2011.xml (working copy) @@ -270,11 +270,10 @@ <entry/> </row> <row> - <?dbhtml bgcolor="#B0B0B0" ?> <entry>19.2</entry> <entry>Exception classes</entry> - <entry>Partial</entry> - <entry>Missing <code>const char*</code> constructors.</entry> + <entry>Y</entry> + <entry/> </row> <row> <entry>19.3</entry> Index: libstdc++-v3/include/std/stdexcept =================================================================== --- libstdc++-v3/include/std/stdexcept (revision 206101) +++ libstdc++-v3/include/std/stdexcept (working copy) @@ -58,9 +58,12 @@ public: /** Takes a character string describing the error. */ - explicit + explicit logic_error(const string& __arg); + explicit + logic_error(const char* __arg); + virtual ~logic_error() _GLIBCXX_USE_NOEXCEPT; /** Returns a C-style character string describing the general cause of @@ -75,6 +78,8 @@ { public: explicit domain_error(const string& __arg); + explicit domain_error(const char* __arg); + virtual ~domain_error() _GLIBCXX_USE_NOEXCEPT; }; @@ -83,6 +88,8 @@ { public: explicit invalid_argument(const string& __arg); + explicit invalid_argument(const char* __arg); + virtual ~invalid_argument() _GLIBCXX_USE_NOEXCEPT; }; @@ -92,6 +99,8 @@ { public: explicit length_error(const string& __arg); + explicit length_error(const char* __arg); + virtual ~length_error() _GLIBCXX_USE_NOEXCEPT; }; @@ -101,6 +110,8 @@ { public: explicit out_of_range(const string& __arg); + explicit out_of_range(const char* __arg); + virtual ~out_of_range() _GLIBCXX_USE_NOEXCEPT; }; @@ -115,9 +126,12 @@ public: /** Takes a character string describing the error. */ - explicit + explicit runtime_error(const string& __arg); + explicit + runtime_error(const char* __arg); + virtual ~runtime_error() _GLIBCXX_USE_NOEXCEPT; /** Returns a C-style character string describing the general cause of @@ -131,6 +145,8 @@ { public: explicit range_error(const string& __arg); + explicit range_error(const char* __arg); + virtual ~range_error() _GLIBCXX_USE_NOEXCEPT; }; @@ -139,6 +155,8 @@ { public: explicit overflow_error(const string& __arg); + explicit overflow_error(const char* __arg); + virtual ~overflow_error() _GLIBCXX_USE_NOEXCEPT; }; @@ -147,6 +165,8 @@ { public: explicit underflow_error(const string& __arg); + explicit underflow_error(const char* __arg); + virtual ~underflow_error() _GLIBCXX_USE_NOEXCEPT; };