stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r680756 - in /stdcxx/branches/4.2.x: include/loc/_codecvt.cc include/loc/_codecvt.h include/loc/_collate.cc include/loc/_collate.h include/loc/_moneypunct.cc include/loc/_moneypunct.h src/codecvt.cpp src/collate.cpp src/wcodecvt.cpp
Date Tue, 29 Jul 2008 18:14:42 GMT
Travis Vitek wrote:
>  
> 
>> Author: sebor
>> Date: Tue Jul 29 09:24:16 2008
>> New Revision: 680756
>>
>> Modified: stdcxx/branches/4.2.x/include/loc/_codecvt.h
>> URL: 
>> http://svn.apache.org/viewvc/stdcxx/branches/4.2.x/include/loc/
>> _codecvt.h?rev=680756&r1=680755&r2=680756&view=diff
>> ===============================================================
>> ===============
>> --- stdcxx/branches/4.2.x/include/loc/_codecvt.h (original)
>> +++ stdcxx/branches/4.2.x/include/loc/_codecvt.h Tue Jul 29 
>> 09:24:16 2008
>> @@ -120,6 +120,8 @@
>>
>>     _EXPLICIT codecvt (_RWSTD_SIZE_T __ref = 0): 
>> _RW::__rw_facet (__ref) { }
>>
>> +    virtual ~codecvt () _RWSTD_ATTRIBUTE_NOTHROW;
>> +
> 
> Are you sure you don't want to be using _RWSTD_DECLARE_NOTHROW here, and
> _RWSTD_DEFINE_NOTHROW with the definition. I only see
> _RWSTD_ATTRIBUTE_NOTHROW being defined for gcc, and that means compile
> error for all other configurations.

Whoops! You're right, this wouldn't work without a definition
of the macro on platforms that don't support the attribute.
I just checked in the change that adds this definition. Sorry
for the our-of-band commit! (I'm working with one local copy
too many...)

> 
> What is the rationale for using the attribute instead of a throw spec? I
> realize that _RWSTD_ATTRIBUTE_NOTHROW will end up being an empty throw
> spec on most platforms, but I don't understand the advantage of using
> the attribute. I see that you use both in this changelist, but I don't
> understand why you selected one over the other.

We don't want _RWSTD_DECLARE_NOTHROW because the macro expands
to throw() on platforms w/o the attribute and we're not allowed
to tighten the throw spec for virtual functions (because users
wouldn't be able declare overrides with looser spec in derived
classes). So the best we can do is affirm the requirement that
dtors don't throw via the attribute, w/o forcing users to do
the same using throw()).

> 
> It seems that the only difference is that, in the worst case, using
> throw specs might require an try/catch block be inserted into the
> definition of the destructor and that should not result in much
> additional code. Of course this would only happen if the compiler cannot
> determine that no exception is thrown by base or member destructors,
> which it should be able to do if they are appropriately decorated.
> 
> That said, it seems that it would be best to decorate the base class
> (_RW::__rw_facet) destructor similarly, regardless of what decoration is
> being used.

Agreed. I have this change scheduled next. I suppose I should
have committed it first. Or added the virtual dtors without
the attribute first, and then decorated them with it in
a separate check in. Yeah, that would have been the way to
do it. I was trying to do unrelated things separately but
clearly didn't do a very good job of it...

> 
>>     // 22,2,1,5,1, p1
>>     result out (state_type& __state,
>>                 const intern_type* __from, const intern_type* 
>> __from_end,
>> @@ -188,7 +190,7 @@
>>
>>     _EXPLICIT codecvt (_RWSTD_SIZE_T = 0);
>>
>> -    virtual ~codecvt ();
>> +    virtual ~codecvt () _RWSTD_ATTRIBUTE_NOTHROW;
> 
> I see no mention of adding this attribute in the changelog. Should there
> be?

Yes, I should have mentioned it. Thanks for the reminder. I added
a mention of this change to the log.

Thanks for keeping an eye on me!

Martin

> 
> Travis
> 


Mime
View raw message