Clean up build warnings of plperl with clang-12+
Hi all,
This is a follow-up of the discussion that happened here:
/messages/by-id/YYoJYO475dsqYhta@paquier.xyz
Recently, we are getting warnings in the build of plperl when using at
least clang-12 because of their new flag -Wcompound-token-split-by-macro,
all the warnings coming from Perl itself.
Upstream has posted a fix about that but it will take time before this
propagates across the buildfarm (five~ animals are now impacted?):
https://www.nntp.perl.org/group/perl.perl5.changes/2021/07/msg57758.html
dangomushi is impacted by that, and I'd like to stick an -Werror in it
as it is proving to be good in detecting incorrect placeholders.
Tom has suggested that we could add -Wno-compound-token-split-by-macro
to take care of the issue on our side, and attached is a patch to do
so.
Any objections? I'd like to get this back-patched.
Thanks,
--
Michael
Attachments:
plperl-clang-warnings.patchtext/x-diff; charset=us-asciiDownload
diff --git a/configure b/configure
index 4ffefe4655..6856ec649d 100755
--- a/configure
+++ b/configure
@@ -6334,6 +6334,52 @@ fi
if test -n "$NOT_THE_CFLAGS"; then
CFLAGS="$CFLAGS -Wno-unused-command-line-argument"
fi
+ # Remove clang 12+'s compound-token-split-by-macro, as this causes a lot
+ # of warnings when building plperl because of Perl. Like previously, test
+ # for the positive form and add the negative form
+ NOT_THE_CFLAGS=""
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wcompound-token-split-by-macro, for NOT_THE_CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wcompound-token-split-by-macro, for NOT_THE_CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${NOT_THE_CFLAGS} -Wcompound-token-split-by-macro"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro=yes
+else
+ pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wcompound_token_split_by_macro" = x"yes"; then
+ NOT_THE_CFLAGS="${NOT_THE_CFLAGS} -Wcompound-token-split-by-macro"
+fi
+
+
+ if test -n "$NOT_THE_CFLAGS"; then
+ CFLAGS="$CFLAGS -Wno-compound-token-split-by-macro"
+ fi
# Similarly disable useless truncation warnings from gcc 8+
NOT_THE_CFLAGS=""
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat-truncation, for NOT_THE_CFLAGS" >&5
diff --git a/configure.ac b/configure.ac
index 44ee3ebe2f..05a00b3e14 100644
--- a/configure.ac
+++ b/configure.ac
@@ -530,6 +530,14 @@ if test "$GCC" = yes -a "$ICC" = no; then
if test -n "$NOT_THE_CFLAGS"; then
CFLAGS="$CFLAGS -Wno-unused-command-line-argument"
fi
+ # Remove clang 12+'s compound-token-split-by-macro, as this causes a lot
+ # of warnings when building plperl because of Perl. Like previously, test
+ # for the positive form and add the negative form
+ NOT_THE_CFLAGS=""
+ PGAC_PROG_CC_VAR_OPT(NOT_THE_CFLAGS, [-Wcompound-token-split-by-macro])
+ if test -n "$NOT_THE_CFLAGS"; then
+ CFLAGS="$CFLAGS -Wno-compound-token-split-by-macro"
+ fi
# Similarly disable useless truncation warnings from gcc 8+
NOT_THE_CFLAGS=""
PGAC_PROG_CC_VAR_OPT(NOT_THE_CFLAGS, [-Wformat-truncation])
On Wed, Nov 10, 2021 at 07:35:21AM +0900, Michael Paquier wrote:
Tom has suggested that we could add -Wno-compound-token-split-by-macro
to take care of the issue on our side, and attached is a patch to do
so.Any objections? I'd like to get this back-patched.
Backpatched this one as of 9ff47ea. That should allow the addition of
-Werror on dangomushi.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
Backpatched this one as of 9ff47ea. That should allow the addition of
-Werror on dangomushi.
Cool. I have also enabled -Werror on florican.
regards, tom lane
Michael Paquier <michael@paquier.xyz> writes:
Backpatched this one as of 9ff47ea. That should allow the addition of
-Werror on dangomushi.
I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't
work because it breaks configure's tests. (I wonder if meson is any
smarter than autoconf about that.) The way I do it on my own
animals is like
if ($branch eq 'HEAD' or $branch ge 'REL9_3')
{
# Add -Werror so we get errors for warning conditions.
# Pre-9.3 PG doesn't compile cleanly with Sierra's cc.
# Can't put this in CFLAGS because it breaks configure,
# so use COPT instead.
$conf{build_env}{COPT} = "-Werror";
}
Guess I could drop the branch check now.
regards, tom lane
On 11/11/21 13:18, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
Backpatched this one as of 9ff47ea. That should allow the addition of
-Werror on dangomushi.I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't
work because it breaks configure's tests. (I wonder if meson is any
smarter than autoconf about that.) The way I do it on my own
animals is likeif ($branch eq 'HEAD' or $branch ge 'REL9_3')
{
# Add -Werror so we get errors for warning conditions.
# Pre-9.3 PG doesn't compile cleanly with Sierra's cc.
# Can't put this in CFLAGS because it breaks configure,
# so use COPT instead.
$conf{build_env}{COPT} = "-Werror";
}Guess I could drop the branch check now.
Wouldn't it be better in any case just to add the clang fix for building
plperl rather than globally?
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
Wouldn't it be better in any case just to add the clang fix for building
plperl rather than globally?
That'd be considerably messier wouldn't it? (And I wonder if the
meson system will be able to do it at all.) If you feel motivated
to make that happen, go for it, but I don't.
regards, tom lane
On Thu, Nov 11, 2021 at 01:18:32PM -0500, Tom Lane wrote:
I see you tried to do that by adding -Werror to CPPFLAGS. That doesn't
work because it breaks configure's tests. (I wonder if meson is any
smarter than autoconf about that.) The way I do it on my own
animals is like
Nya. (Japanized Meh)
if ($branch eq 'HEAD' or $branch ge 'REL9_3')
{
# Add -Werror so we get errors for warning conditions.
# Pre-9.3 PG doesn't compile cleanly with Sierra's cc.
# Can't put this in CFLAGS because it breaks configure,
# so use COPT instead.
$conf{build_env}{COPT} = "-Werror";
}Guess I could drop the branch check now.
Thanks. I'll switch to something like that.
--
Michael
On Thu, Nov 11, 2021 at 03:51:35PM -0500, Tom Lane wrote:
That'd be considerably messier wouldn't it?
Yes, that would be a bit messier. For example, we could do that with
something saved in Makefile.global.in by ./configure that plperl feeds
on to add this extra CFLAGS in its own local Makefile.
(And I wonder if the
meson system will be able to do it at all.) If you feel motivated
to make that happen, go for it, but I don't.
Neither am I. 9ff47ea is the least invasive solution, at least.
--
Michael