Clean up build warnings of plperl with clang-12+

Started by Michael Paquierover 4 years ago8 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

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+54-0
#2Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#1)
Re: Clean up build warnings of plperl with clang-12+

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Clean up build warnings of plperl with clang-12+

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Clean up build warnings of plperl with clang-12+

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

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#4)
Re: Clean up build warnings of plperl with clang-12+

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 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.

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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#5)
Re: Clean up build warnings of plperl with clang-12+

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

#7Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#4)
Re: Clean up build warnings of plperl with clang-12+

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

#8Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#6)
Re: Clean up build warnings of plperl with clang-12+

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