This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/6] float128: Add strtof128, wcstof128, and related functions.


On Fri, 26 May 2017, Gabriel F. T. Gomes wrote:

> diff --git a/NEWS b/NEWS
> index 8cb17cc..39e15fa 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -69,6 +69,10 @@ Version 2.26
>  * The function strfromf128, from ISO/IEC TS 18661-3:2015, is added to libc.
>    It converts a _Float128 value into string.
>  
> +* The function strtof128, from ISO/IEC TS 18661-3:2015, is added to libc.
> +  It converts a string into a floating-point number of _Float128 type.  Its
> +  wide character counterpart, wsctof128, is added as a GNU extension.

Same comment as before applies about having a NEWS entry saying what 
systems a feature is supported for, only when that is a nonempty set.

> @@ -235,6 +242,14 @@ extern long double strtold_l (const char *__restrict __nptr,
>  			      char **__restrict __endptr,
>  			      __locale_t __loc)
>       __THROW __nonnull ((1, 3));
> +
> +# if __HAVE_FLOAT128 && __GLIBC_USE (IEC_60559_TYPES_EXT)
> +extern _Float128 strtof128_l (const char *__restrict __nptr,
> +			      char **__restrict __endptr,
> +			      __locale_t __loc)
> +     __THROW __nonnull ((1, 3));
> +# endif
> +
>  #endif /* GNU */

As you're inside a __USE_GNU conditional, the && __GLIBC_USE 
(IEC_60559_TYPES_EXT) is not necessary here.

> diff --git a/stdlib/tst-strtod.h b/stdlib/tst-strtod.h
> index bf5f901..d862a67 100644
> --- a/stdlib/tst-strtod.h
> +++ b/stdlib/tst-strtod.h
> @@ -21,11 +21,36 @@
>  
>  #define FSTRLENMAX 128
>  
> +#include <bits/floatn.h>
> +
> +#if !__GNUC_PREREQ(7,0)
> +# define F128 Q
> +#endif

Can we avoid this duplication of backwards compatibility with old GCC 
versions?  E.g. use __f128 () to get the required suffix.

> +#define _GEN(mfunc, type, ...) _GENx(_GEN_ ## type, mfunc, type, __VA_ARGS__)
> +#define _GENx(mmfunc, mfunc, type, ...) mmfunc (mfunc, type, __VA_ARGS__)
> +#define _DO(mfunc, type, ...) _DOx(_DO_ ## type, mfunc, ##__VA_ARGS__)
> +#define _DOx(mmfunc, mfunc, ...) mmfunc (mfunc, ##__VA_ARGS__)

Missing spaces before '(' in macro calls.

> diff --git a/sysdeps/ieee754/float128/mpn2float128.c b/sysdeps/ieee754/float128/mpn2float128.c
> new file mode 100644
> index 0000000..ad2e699
> --- /dev/null
> +++ b/sysdeps/ieee754/float128/mpn2float128.c
> @@ -0,0 +1,53 @@
> +/* Copyright (C) 2017 Free Software Foundation, Inc.

Descriptive comment needed before copyright notice at the start of each 
new file.

> diff --git a/sysdeps/ieee754/float128/wcstof128.c b/sysdeps/ieee754/float128/wcstof128.c
> new file mode 100644
> index 0000000..d87eff9
> --- /dev/null
> +++ b/sysdeps/ieee754/float128/wcstof128.c
> @@ -0,0 +1,29 @@
> +/* Copyright (C) 2017 Free Software Foundation, Inc.

Likewise.

> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
> index 79be9fc..ba89c22 100644
> --- a/wcsmbs/wchar.h
> +++ b/wcsmbs/wchar.h
> @@ -468,6 +468,15 @@ __extension__
>  extern unsigned long long int wcstouq (const wchar_t *__restrict __nptr,
>  				       wchar_t **__restrict __endptr,
>  				       int __base) __THROW;
> +
> +/* Include _Float128 variants of strtof128 if available.  */
> +#include <bits/floatn.h>
> +
> +#if __HAVE_FLOAT128 && __GLIBC_USE (IEC_60559_TYPES_EXT)
> +extern _Float128 wcstof128 (const wchar_t *__restrict __nptr,
> +			    wchar_t **__restrict __endptr) __THROW;
> +#endif

This seems like the wrong place in the header.  Just after wcstof/wcstold 
would be better (with __HAVE_FLOAT128 && defined __USE_GNU as the 
conditional).  And with <bits/floatn.h> included along with the other 
<bits/*.h> includes at the start of the header.

> @@ -518,6 +527,13 @@ extern float wcstof_l (const wchar_t *__restrict __nptr,
>  extern long double wcstold_l (const wchar_t *__restrict __nptr,
>  			      wchar_t **__restrict __endptr,
>  			      __locale_t __loc) __THROW;
> +
> +#if __HAVE_FLOAT128 && __GLIBC_USE (IEC_60559_TYPES_EXT)
> +extern _Float128 wcstof128_l (const wchar_t *__restrict __nptr,
> +			      wchar_t **__restrict __endptr,
> +			      __locale_t __loc) __THROW;
> +#endif

No need for __GLIBC_USE (IEC_60559_TYPES_EXT) inside __USE_GNU 
conditional.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]