This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: [libcxx+newlib] nexttoward{f,l,} shims for _LDBL_EQ_DBL systems
- From: JF Bastien <jfb at chromium dot org>
- To: Jonathan Roelofs <jonathan at codesourcery dot com>
- Cc: Craig Howland <howland at lgsinnovations dot com>, "newlib at sourceware dot org" <newlib at sourceware dot org>
- Date: Tue, 2 Dec 2014 10:45:21 -0800
- Subject: Re: [libcxx+newlib] nexttoward{f,l,} shims for _LDBL_EQ_DBL systems
- Authentication-results: sourceware.org; auth=none
- References: <5421D931 dot 905 at codesourcery dot com> <54221BC3 dot 9020103 at LGSInnovations dot com> <5422F390 dot 1060401 at codesourcery dot com>
Is this change ready to land?
On Wed, Sep 24, 2014 at 9:38 AM, Jonathan Roelofs
<jonathan@codesourcery.com> wrote:
> Craig,
>
> Thanks for the review!
>
> On 9/23/14 7:17 PM, Craig Howland wrote:
>>
>> On 09/23/2014 04:33 PM, Jonathan Roelofs wrote:
>>>
>>> Please see the attached patch.
>>>
>> Needs a ChangeLog entry. Missing the patch for libm/common/Makefile.am.
>
> Oops. I'm not very familiar with Newlib processes, so please let me know if
> I'm missing other things that you'd normally expect from such an upstream
> submission.... For example, some of my co-workers reminded me to also update
> COPYING.NEWLIB, and add copyright headers to the new files.
>>
>>
>> Operationally, all looks OK, but aesthetically, the use of casting is
>> inconsistent. For example:
>>
>> +long double
>> +nexttowardl (long double x, long double y)
>> +{
>> + return nextafter((double)x, (double)y);
>> +}
>>
>> The x and y arguments are cast to double to pass to nextafter(), but the
>> return
>> value is not cast to long double. It works OK, but could be confusing.
>> Are the
>> casts intended to highlight the spots where the double/long double
>> equality is
>> required?
>
> Yeah, that was my intent.
>>
>> If so, it would probably be better to either cast in all places where
>> it is theoretically needed, or to just leave them out. The other existing
>> functions of this style left all of them out (i.e. no casts from ld to d
>> or d to
>> ld)--see, for example, libm/common/nextafterl.c. I suggest that the
>> precedent
>> is the better way.
>
> Agreed, consistency is probably better.
>
> Attached is a new version of this patch with these things addressed, and
> I've added another similar shim for log2l.
>
>
> Cheers,
>
> Jon
>>
>>
>> Craig
>
>
> --
> Jon Roelofs
> jonathan@codesourcery.com
> CodeSourcery / Mentor Embedded