On Dec 29, 2013, at 3:24 PM, Howard Hinnant <[email protected]> wrote:

> On Dec 29, 2013, at 11:53 AM, Marshall Clow <[email protected]> wrote:
> 
>> Here’s a patch for review.
>> It only deals with “pow”, but the same techniques work on all the other 
>> calls.
>> 
>> [ I chose “pow” because there’s a “pow” in <complex>, and we have to not 
>> step on that. ]
>> 
>> Anyway - if we have rvalue references, then we’ll use them. If not, then we 
>> pass by value.
>> We can’t pass by const &, because the conversion operator might not be const.
>> We can’t pass by non-const &, because that would fail on literal values.
>> 
>> The exception specification depends on conversion to floating point.
>> Note that pow (double, double) is NOT noexcept - because that’s defined 
>> elsewhere :-(, and we can’t change that.
>> [ Even though it will never throw, we can’t mark it as noexcept ]
>> 
>> noexcept is QOI - it’s not specified in the standard.
>> 
> 
> This is looking good.  Thanks much for all of the work you've put into this!
> 
> A counter patch is enclosed below, closely related to the patch you have.
> 
> Notable differences:
> 
> 1.  __numeric_type has been demoted to an implementation detail of __promote. 
>  The motivation for this is to simply the code in <cmath> as much as possible 
> since it has to be repeated for each math function.
> 
> 2.  __promote<A1, A2, ...> now has a nested "type" that may be computed from 
> a user-defined type X with an implicit, non-ambiguous conversion to an 
> arithmetic type (just like __numeric_type).
> 
> 3.  __promote<A1, A2, ...>::type now does not exist if the conversion for 
> each A does not exist (like __numeric_type, but expanded to multiple 
> arguments now).  This enables __promote to act as its own enable_if, 
> simplifying the return type of most of the math functions.
> 
> 4.  If __promote<A1, A2, ...> also has a nested bool: value.  This is true if 
> the nested type exists, else it is false.  This is handy for enable_if'ing 
> isnan, which returns bool instead of the "generic double".
> 
> 5.  __promote<A1> (single argument only) will contain another const static 
> bool:  __does_not_throw, which is true if the conversion from A1 to 
> __promote<A1>::type does not throw an exception.  This simplifies the 
> noexcept spec out at the math function level.
> 
> 6.  Added perfect forwarding to the return statement in pow.  This makes sure 
> things work if the conversion operator has an "rvalue-this" qualification.
> 
> 7.  Added remove_reference to the static_assert within pow.  This 
> static_assert is designed to prevent infinite recursion due to a logic bug in 
> this design.  It basically says:  Don't call this templated thing again!
> 
> 8.  Because isnan is a little different, I've prototyped that too.  Now isnan 
> has ordinary float, double and long double overloads (like pow, but in the 
> global namespace).  And the templated isnan is moved inside of namespace std. 
>  Because it always returns bool, it has an explicit enable_if based on 
> __promote<_A1>::value (does the conversion exist), instead of on whether or 
> not the nested "type" exists in __promote.
> 
> Tests seem to be passing.  But this needs careful review from all interested 
> parties.

This LGTM.
I think it addresses all the issues that we’ve seen.

Sorry it took me so long to respond.

— Marshall

P.S.    It doesn’t actually solve all of PR18218; but it gives us the tools 
needed to solve the rest of the bug.


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to