Skip Menu |
Report information
Id: 130726
Status: pending release
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: hv <hv [at] crypt.org>
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: low
Type: core
Perl Version: 5.18.2
Fixed In: (no value)



Date: Mon, 06 Feb 2017 14:10:15 +0000
CC: hv [...] crypt.org
To: perlbug [...] perl.org
Subject: semicolons on *_DIAG_{IGNORE,RESTORE}
From: hv [...] crypt.org
This is a bug report for perl from hv@crypt.org, generated with the help of perlbug 1.39 running under perl 5.18.2. ----------------------------------------------------------------- [Please describe your issue here] From perl.h: [...] * which makes compilers mildly cranky. Therefore at file level one * should use the GCC_DIAG_IGNORE and GCC_DIAG_RESTORE_FILE *without* * the semicolons. % # probably correct % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?!\s*;)' | wc -l 17 % # probably incorrect % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?=\s*;)' | wc -l 70 % Is it worth fixing these wholesale? (Prompted by surprise at d7f10d9369 from James Keenan earlier today.) [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.18.2: Configured by Debian Project at Tue Mar 1 17:12:59 UTC 2016. Summary of my perl5 (revision 5 version 18 subversion 2) configuration: Platform: osname=linux, osvers=3.13.0-79-generic, archname=x86_64-linux-gnu-thread-multi uname='linux lgw01-37 3.13.0-79-generic #123-ubuntu smp fri feb 19 14:27:58 utc 2016 x86_64 x86_64 x86_64 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Dldflags= -Wl,-Bsymbolic-functions -Wl,-z,relro -Dlddlflags=-shared -Wl,-Bsymbolic-functions -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.18 -Darchlib=/usr/lib/perl/5.18 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.18.2 -Dsitearch=/usr/local/lib/perl/5.18.2 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.18.2 -des' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g', cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='4.8.2', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=, so=so, useshrplib=true, libperl=libperl.so.5.18.2 gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector' Locally applied patches: DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN. DEBPKG:debian/db_file_ver - http://bugs.debian.org/340047 Remove overly restrictive DB_File version check. DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information. DEBPKG:debian/enc2xs_inc - http://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories. DEBPKG:debian/errno_ver - http://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes. DEBPKG:debian/libperl_embed_doc - http://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking DEBPKG:fixes/respect_umask - Respect umask during installation DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories DEBPKG:debian/extutils_set_libperl_path - EU:MM: Set location of libperl.a to /usr/lib DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor DEBPKG:debian/prefix_changes - Fiddle with *PREFIX and variables written to the makefile DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets. DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor. DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy. DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable. DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian DEBPKG:debian/module_build_man_extensions - http://bugs.debian.org/479460 Adjust Module::Build manual page extensions for the Debian Perl policy DEBPKG:debian/prune_libs - http://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need. DEBPKG:fixes/net_smtp_docs - [rt.cpan.org #36038] http://bugs.debian.org/100195 Document the Net::SMTP 'Port' option DEBPKG:debian/perlivp - http://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local DEBPKG:debian/cpanplus_definstalldirs - http://bugs.debian.org/533707 Configure CPANPLUS to use the site directories by default. DEBPKG:debian/cpanplus_config_path - Save local versions of CPANPLUS::Config::System into /etc/perl. DEBPKG:debian/deprecate-with-apt - http://bugs.debian.org/702096 Point users to Debian packages of deprecated core modules DEBPKG:debian/squelch-locale-warnings - http://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository DEBPKG:debian/patchlevel - http://bugs.debian.org/567489 List packaged patches for 5.18.2-2ubuntu1.1 in patchlevel.h DEBPKG:debian/skip-kfreebsd-crash - http://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD DEBPKG:fixes/document_makemaker_ccflags - http://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags} DEBPKG:debian/find_html2text - http://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text DEBPKG:debian/hurd_test_skip_stack - http://bugs.debian.org/650175 Disable failing GNU/Hurd tests dist/threads/t/stack.t DEBPKG:fixes/manpage_name_Test-Harness - http://bugs.debian.org/650451 [rt.cpan.org #73399] cpan/Test-Harness: add NAME headings in modules with POD DEBPKG:debian/makemaker-pasthru - http://bugs.debian.org/660195 [rt.cpan.org #28632] Make EU::MM pass LD through to recursive Makefile.PL invocations DEBPKG:debian/perl5db-x-terminal-emulator.patch - http://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl DEBPKG:debian/cpan-missing-site-dirs - http://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected DEBPKG:fixes/net_ftp_failed_command - [rt.cpan.org #37700] http://bugs.debian.org/491062 Net::FTP: cope gracefully with a failed command DEBPKG:fixes/perlbug-patchlist - [3541c11] http://bugs.debian.org/710842 [perl #118433] Make perlbug look up the list of local patches at run time DEBPKG:fixes/module_metadata_security_doc - [68cdd4b] CVE-2013-1437 documentation fix DEBPKG:fixes/module_metadata_taint_fix - [bff978f] http://bugs.debian.org/722210 [rt.cpan.org #88576] untaint version, if needed, in Module::Metadata DEBPKG:fixes/IPC-SysV-spelling - http://bugs.debian.org/730558 [rt.cpan.org #86736] Fix spelling of IPC_CREAT in IPC-SysV documentation DEBPKG:fixes/fix-undef-source - DEBPKG:fixes/CVE-2013-7422.patch - [PATCH] [perl #119505] Segfault from bad backreference DEBPKG:fixes/CVE-2014-4330.patch - [PATCH] don't recurse infinitely in Data::Dumper DEBPKG:fixes/CVE-2016-2381.patch - [PATCH 1/2] remove duplicate environment variables from environ --- @INC for perl 5.18.2: /etc/perl /usr/local/lib/perl/5.18.2 /usr/local/share/perl/5.18.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.18 /usr/share/perl/5.18 /usr/local/lib/site_perl . --- Environment for perl 5.18.2: HOME=/home/hv LANG=C LANGUAGE=en_GB:en LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=<elided> PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Mon, 06 Feb 2017 14:17:33 GMT, hv wrote: Show quoted text
> This is a bug report for perl from hv@crypt.org, > generated with the help of perlbug 1.39 running under perl 5.18.2. > > ----------------------------------------------------------------- > [Please describe your issue here] > > From perl.h: > [...] > * which makes compilers mildly cranky. Therefore at file level one > * should use the GCC_DIAG_IGNORE and GCC_DIAG_RESTORE_FILE *without* > * the semicolons. > > % # probably correct > % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?!\s*;)' | wc -l > 17 > % # probably incorrect > % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?=\s*;)' | wc -l > 70 > % > > Is it worth fixing these wholesale? > > (Prompted by surprise at d7f10d9369 from James Keenan earlier today.) >
Errrrm ... probably not (yet). I was playing around with the approach being explored by Andy Lester -- compile with clang under '-Weverything'. I came up with exactly one instance of '-Wextra-semi' in the entire core distribution -- the one in threads.xs. A one-character correction, reviewed by threads maintainer Jerry Hedden. Extremely low hanging fruit. But I can tell that many of the other warnings generated under '-Weverything' need more careful thought before we touch them. I don't have much experience in this area. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.3k
On Tue, 07 Feb 2017 09:55:26 -0800, jkeenan wrote: Show quoted text
> On Mon, 06 Feb 2017 14:17:33 GMT, hv wrote:
> > This is a bug report for perl from hv@crypt.org, > > generated with the help of perlbug 1.39 running under perl 5.18.2. > > > > ----------------------------------------------------------------- > > [Please describe your issue here] > > > > From perl.h: > > [...] > > * which makes compilers mildly cranky. Therefore at file level one > > * should use the GCC_DIAG_IGNORE and GCC_DIAG_RESTORE_FILE *without* > > * the semicolons. > > > > % # probably correct > > % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?!\s*;)' | wc -l > > 17 > > % # probably incorrect > > % grep -Pr '(GCC|CLANG)_DIAG_(IGNORE\(.*?\)|RESTORE)(?=\s*;)' | wc -l > > 70 > > % > > > > Is it worth fixing these wholesale? > > > > (Prompted by surprise at d7f10d9369 from James Keenan earlier today.) > >
> > Errrrm ... probably not (yet). > > I was playing around with the approach being explored by Andy Lester > -- compile with clang under '-Weverything'. I came up with exactly > one instance of '-Wextra-semi' in the entire core distribution -- the > one in threads.xs. A one-character correction, reviewed by threads > maintainer Jerry Hedden. Extremely low hanging fruit. > > But I can tell that many of the other warnings generated under '- > Weverything' need more careful thought before we touch them. I don't > have much experience in this area.
I'm not talking about wholesale fixes for -Weverything, just about wholesale fixes to the correct usage of these particular 4 macros - correct usage that we've already gone to the bother of documenting in perl.h but apparently mostly ignored for actual use. If avoiding the semicolon is the correct thing to do, then it should be "extremely low-hanging fruit" everywhere - I think due diligence would consist at most in checking that they're preceded and followed by something well-formed. It also feels more sane to fix at least the *.[ch] examples in one commit than in 70-odd separate ones. An alternative approach would be to decide that we don't care - perhaps that we gain more from visual consistency with other statement-type macros in the source than we do from -Weverything-cleanliness - in which case we'd probably be better off changing the perl.h recommendation and consistently having the semicolon everywhere. The mess of a halfway house is what loses us the most. Hugo
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130726] semicolons on *_DIAG_{IGNORE,RESTORE}
To: perl5-porters [...] perl.org
Date: Sat, 16 Dec 2017 07:15:55 +0000
Download (untitled) / with headers
text/plain 269b
We can't in practice prevent people using semicolons, so semicolon-friendly macro definitions are to be preferred. This necessitates using distinct macros for statement and declaration contexts. Implemented in commit 7347ee5481c77d2fdb023e0a1c19943d039ecc86. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 481b
On Fri, 15 Dec 2017 23:16:09 -0800, zefram@fysh.org wrote: Show quoted text
> We can't in practice prevent people using semicolons, so > semicolon-friendly macro definitions are to be preferred. This > necessitates using distinct macros for statement and declaration contexts. > Implemented in commit 7347ee5481c77d2fdb023e0a1c19943d039ecc86.
Looks good to me; the only matches now updated versions of my original greps are on the macro declarations themselves. Setting as pending release. Hugo


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org