Fix localedata test builds with latest GCC
Commit Message
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.
Comments
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.
@@ -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)