Fix localedata test builds with latest GCC

  While testing the latest glibc sources with the latest (ToT) GCC, I ran
into some build failures (during testing, not during the glibc build).
This patch fixes the build failures in the localedata subdirectory.  I
believe these started failing with this GCC patch:

	https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00537.html

It does not look like older GCC's complain about not understanding this
option so there does not seem to be any need to conditionalize its usage.

Ok to checkin?

Steve Ellcey
sellcey@cavium.com


2017-07-19  Steve Ellcey  <sellcey@cavium.com>

	* localedata/Makefile (CFLAGS-tst_iswalnum.c, CFLAGS-tst_iswalpha.c
	CFLAGS-tst_iswcntrl.c, CFLAGS-tst_iswdigit.c, CFLAGS-tst_iswgraph.c,
	CFLAGS-tst_iswlower.c, CFLAGS-tst_iswprint.c, CFLAGS-tst_iswpunct.c,
	CFLAGS-tst_iswspace.c, CFLAGS-tst_iswupper.c, CFLAGS-tst_iswxdigit.c,
	CFLAGS-tst_towlower.c, CFLAGS-tst_towupper.c): New macros.
  

On Wed, Jul 19, 2017 at 3:14 PM, Steve Ellcey <sellcey@cavium.com> wrote:
> While testing the latest glibc sources with the latest (ToT) GCC, I ran
> into some build failures (during testing, not during the glibc build).
> This patch fixes the build failures in the localedata subdirectory.  I
> believe these started failing with this GCC patch:
>
>         https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00537.html
>
> It does not look like older GCC's complain about not understanding this
> option so there does not seem to be any need to conditionalize its usage.

I can't conveniently experiment with GCC 8.x right now, but this seems
like the right general idea.

zw
  

On Wed, 2017-07-19 at 15:20 -0500, Zack Weinberg wrote:
> On Wed, Jul 19, 2017 at 3:14 PM, Steve Ellcey <sellcey@cavium.com>
> wrote:
> > 
> > While testing the latest glibc sources with the latest (ToT) GCC, I ran
> > into some build failures (during testing, not during the glibc build).
> > This patch fixes the build failures in the localedata subdirectory.  I
> > believe these started failing with this GCC patch:
> > 
> >         https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00537.html
> > 
> > It does not look like older GCC's complain about not understanding this
> > option so there does not seem to be any need to conditionalize its usage.
> I can't conveniently experiment with GCC 8.x right now, but this seems
> like the right general idea.
> 
> zw

Does anyone else have any comments, pro or con, on this patch?  I would
like to check it in today for 2.26 if there are no objections, but it
is certainly not a showstopper if there are objections.

Steve Ellcey
sellcey@cavium.com
  

On 07/19/2017 04:14 PM, Steve Ellcey wrote:
> While testing the latest glibc sources with the latest (ToT) GCC, I ran
> into some build failures (during testing, not during the glibc build).
> This patch fixes the build failures in the localedata subdirectory.  I
> believe these started failing with this GCC patch:
> 
> 	https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00537.html
> 
> It does not look like older GCC's complain about not understanding this
> option so there does not seem to be any need to conditionalize its usage.
> 
> Ok to checkin?

OK.

This looks good to me. The alternative of using the DIAG macros is not
feasibly nor easy, so I'm OK to apply the error removal in a blanket
fashion like this for each of the files.

If you filed an upstream gcc PR please include the PR in the comment.

> Steve Ellcey
> sellcey@cavium.com
> 
> 
> 2017-07-19  Steve Ellcey  <sellcey@cavium.com>
> 
> 	* localedata/Makefile (CFLAGS-tst_iswalnum.c, CFLAGS-tst_iswalpha.c
> 	CFLAGS-tst_iswcntrl.c, CFLAGS-tst_iswdigit.c, CFLAGS-tst_iswgraph.c,
> 	CFLAGS-tst_iswlower.c, CFLAGS-tst_iswprint.c, CFLAGS-tst_iswpunct.c,
> 	CFLAGS-tst_iswspace.c, CFLAGS-tst_iswupper.c, CFLAGS-tst_iswxdigit.c,
> 	CFLAGS-tst_towlower.c, CFLAGS-tst_towupper.c): New macros.
> 
> 
> diff --git a/localedata/Makefile b/localedata/Makefile
> index 47ca39d..20c5921 100644
> --- a/localedata/Makefile
> +++ b/localedata/Makefile
> @@ -122,6 +122,21 @@ $(inst_i18ndir)/charmaps/%.gz: charmaps/% $(+force)
>  # Install the locale source files in the appropriate directory.
>  $(inst_i18ndir)/locales/%: locales/% $(+force); $(do-install)
>  
> +# These tests use multistatement macros from tests-mbwc/tst_funcs.h
> +# and will not compile with GCC 8.1 without the warning turned off.
> +CFLAGS-tst_iswalnum.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswalpha.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswcntrl.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswdigit.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswgraph.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswlower.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswprint.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswpunct.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswspace.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswupper.c  = -Wno-multistatement-macros
> +CFLAGS-tst_iswxdigit.c = -Wno-multistatement-macros
> +CFLAGS-tst_towlower.c  = -Wno-multistatement-macros
> +CFLAGS-tst_towupper.c  = -Wno-multistatement-macros
>  
>  ifeq ($(run-built-tests),yes)
>  generated-dirs += $(LOCALES)
>
  

On Wed, 19 Jul 2017, Steve Ellcey wrote:

> While testing the latest glibc sources with the latest (ToT) GCC, I ran
> into some build failures (during testing, not during the glibc build).
> This patch fixes the build failures in the localedata subdirectory.  I
> believe these started failing with this GCC patch:
> 
> 	https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00537.html

The warnings you quoted look very much like false positives to me, i.e. 
something that should be fixed in GCC not worked around in glibc.

Specifically, they look quite like the issue described in 
<https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01827.html>.  Does that 
patch avoid the need for these -Wno- options?  If so, that indicates these 
-Wno- options should be removed.  If not, that indicates engaging with 
that thread would be appropriate to explain how the glibc case differs 
(unless there are cases in glibc that can be shown *not* to be false 
positives, i.e. where the code suggests a macro expansion is entirely 
within an "if" etc. but only part of it is).

> +# These tests use multistatement macros from tests-mbwc/tst_funcs.h
> +# and will not compile with GCC 8.1 without the warning turned off.

A comment referencing GCC 8.1 can't possibly be correct here yet; that's 
making a prediction about a version of GCC from April 2018, and so is 
misleading by suggesting something applies to a future release that may or 
may not apply to that release.  All GCC 8 development versions at present 
are 8.0.