5.23.8 + "Assuming NOT a POSIX" causes spurious warning in PPIx::Regexp::Token::Literal #15190

p5pRT · 2016-02-21T02:36:56Z

Migrated from rt.perl.org#127581 (status was 'resolved')

Searchable as RT127581$

p5pRT · 2016-02-21T02:36:56Z

From @kentfredric

This is a bug report for perl from kentnl@cpan.org,
generated with the help of perlbug 1.40 running under perl 5.23.8.

The following line of code raises a warning, despite looking like legal use of POSIX character classes:

https://metacpan.org/source/WYANT/PPIx-Regexp-0.047/lib/PPIx/Regexp/Token/Literal.pm#L242

This cascades into Perl::Critic::Pulp where that warning becomes an error:

http://www.cpantesters.org/cpan/report/3df914e0-d0cb-11e5-82fe-70d067b6c32e

The code in question doesn't seem obviously wrong:

c [][[:alpha:]\@\\^_?] | # control characters

Flags:
category=core
severity=low

Site configuration information for perl 5.23.8:

Configured by kent at Sun Feb 21 13:35:32 NZDT 2016.

Summary of my perl5 (revision 5 version 23 subversion 8) configuration:

Platform:
osname=linux, osvers=4.4.0-gentoo-r1, archname=x86_64-linux
uname='linux katipo2 4.4.0-gentoo-r1 #33 smp preempt fri jan 29 01:03:50 nzdt 2016 x86_64 intel(r) core(tm) i5-2410m cpu @ 2.30ghz genuineintel gnulinux '
config_args='-de -Dprefix=/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc -Doptimize= -fno-stack-protector -O3 -march=native -mtune=native -Dman1dir=none -Dman3dir=none -Dusedevel -Accflags= -fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -Aldflags= -fno-stack-protector -Aeval:scriptdir=/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/bin'
hint=recommended, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
use64bitint=define, use64bitall=define, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize=' -fno-stack-protector -O3 -march=native -mtune=native',
cppflags='-fno-stack-protector -DPERL_HASH_FUNC_SDBM -DPERL_DISABLE_PMC -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong'
ccversion='', gccversion='5.3.0', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='cc', ldflags =' -fno-stack-protector -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/include-fixed /usr/lib /usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64
libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
libc=libc-2.22.so, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.22'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -fno-stack-protector -O3 -march=native -mtune=native -L/usr/local/lib -fstack-protector-strong'

@INC for perl 5.23.8:
/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/lib/site_perl/5.23.8/x86_64-linux
/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/lib/site_perl/5.23.8
/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/lib/5.23.8/x86_64-linux
/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/lib/5.23.8
.

Environment for perl 5.23.8:
HOME=/home/kent
LANG=en_NZ.UTF8
LANGUAGE (unset)
LC_CTYPE=en_NZ.UTF8
LC_TIME=en_NZ.UTF8
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/kent/perl5/perlbrew/bin:/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/bin:/home/kent/.perl6/2013.04/bin:/home/kent/.gem/ruby/1.8/bin/:/home/kent/.rvm/gems/ruby-2.1.2/bin:/home/kent/.rvm/gems/ruby-2.1.2@global/bin:/home/kent/.rvm/rubies/ruby-2.1.2/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/5.3.0:/usr/games/bin:/home/kent/.rvm/bin:/home/kent/.rvm/bin
PERLBREW_BASHRC_VERSION=0.74
PERLBREW_HOME=/home/kent/.perlbrew
PERLBREW_MANPATH=/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/man
PERLBREW_PATH=/home/kent/perl5/perlbrew/bin:/home/kent/perl5/perlbrew/perls/5.23.8-nossp-sdbm-nopmc/bin
PERLBREW_PERL=5.23.8-nossp-sdbm-nopmc
PERLBREW_ROOT=/home/kent/perl5/perlbrew
PERLBREW_VERSION=0.74
PERL_BADLANG (unset)
SHELL=/bin/bash

p5pRT · 2016-02-21T04:55:36Z

From @tonycoz

On Sat Feb 20 18:36:56 2016, kentfredric wrote:

c [][[:alpha:]\@\\^_?] | # control characters

$ ./perl -Ilib -We 'qr/[][[:alpha:]]/'
Assuming NOT a POSIX class since there must be a starting ':' in regex; marked by <-- HERE in m/[][ <-- HERE [:alpha:]]/ at -e line 1.
$ ./perl -Ilib -We 'qr/[[:alpha:]]/'
$

Tony

p5pRT · 2016-02-21T04:55:37Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2016-02-21T05:42:05Z

From @cpansprout

On Sat Feb 20 18:36:56 2016, kentfredric wrote:

This is a bug report for perl from kentnl@cpan.org,
generated with the help of perlbug 1.40 running under perl 5.23.8.

-----------------------------------------------------------------

The following line of code raises a warning, despite looking like
legal use of POSIX character classes:

https://metacpan.org/source/WYANT/PPIx-Regexp-
0.047/lib/PPIx/Regexp/Token/Literal.pm#L242

This cascades into Perl::Critic::Pulp where that warning becomes an
error:

http://www.cpantesters.org/cpan/report/3df914e0-d0cb-11e5-82fe-
70d067b6c32e

The code in question doesn't seem obviously wrong:

c [][[:alpha:]\@\\^_?] | # control characters

In fact, I don’t see *anything* wrong with it. It looks as though this new warning is always going to trigger false positives. I can’t see any way around that (except by removing the warning).

--

Father Chrysostomos

p5pRT · 2016-02-21T16:17:37Z

From @khwilliamson

On 02/20/2016 10:42 PM, Father Chrysostomos via RT wrote:

On Sat Feb 20 18:36:56 2016, kentfredric wrote:

This is a bug report for perl from kentnl@cpan.org,
generated with the help of perlbug 1.40 running under perl 5.23.8.

-----------------------------------------------------------------

The following line of code raises a warning, despite looking like
legal use of POSIX character classes:

https://metacpan.org/source/WYANT/PPIx-Regexp-
0.047/lib/PPIx/Regexp/Token/Literal.pm#L242

This cascades into Perl::Critic::Pulp where that warning becomes an
error:

http://www.cpantesters.org/cpan/report/3df914e0-d0cb-11e5-82fe-
70d067b6c32e

The code in question doesn't seem obviously wrong:

c [][[:alpha:]\@\\^_?] | # control characters

In fact, I don’t see *anything* wrong with it. It looks as though this new warning is always going to trigger false positives. I can’t see any way around that (except by removing the warning).

Actually, it is fairly easy to get rid of this entire class of false
positive.

The changes I made do not change what actually get compiled. So this
warning was wrong, the [:alpha:] was parsed correctly. The warning was
actually saying that this portion '[[:alpha:]' was not a valid posix
class. It's easy to suppress the warning if that region of text
overlaps a region that had a proper posix class. And I'll do that over
the next few days. Thus the warning can be made to appear only when
something isn't parsed as a posix class, but looks like it was intended
to be one.

p5pRT · 2016-03-01T16:44:03Z

From @trwyant

This warning is suppressed in PPIx-Regexp 0.048, which went to PAUSE yesterday. There's no test coverage tool that I know of to test regex coverage, but I believe the regex was functioning correctly.

What happened was that the lazy way to include a right square bracket in a bracketed character class is to put it first. Having square brackets on the brain, I put the left square bracket next, and followed with the POSIX class. The fix was to shuffle the \@ before the POSIX class, as suggested by perldiag. I admit that I grumbled to myself a little over this, but on reflection it's no different than shuffling that right square bracket to the front.

As I noted to a correspondent on this issue, Perl is so overloaded that it is fairly easy to write something that is valid Perl but nevertheless quite different than the author's intention, and warnings like this have a long history. But if you will allow the opinion of someone whose normal involvement with the porters' mailing list is reading (with profound thanks!) SawyerX's summaries, I personally would appreciate the suppression of the false positive (which I believe this is) if it can be done without eliminating the message completely.

With thanks to all of you for the continuous improvement of Perl 5,
Tom Wyant

p5pRT · 2016-03-01T17:06:31Z

From @khwilliamson

This has been fixed by b7c50ab

I forgot to flesh out the commit message, so here is some more detail:

Basically the fix is to delay the warning message until we are sure it was not a false positive. In the failure from this ticket:

/[][[:alpha:]]

it sees the 2nd '[' (the 3rd character within the pattern, immediately after a ']') and starts looking for a posix class. It finds one at that position, but with an apparent typo: '[[:alpha:]. So it raised a warning. Now, instead, it saves the warning. Then it starts parsing at the next '[' and sees a legitimate class '[:alpha:]', and it then compiled that, but the warning had already escaped. With this commit, the warning will be cleared if a later legitimate class is found that overlaps the area the warning was about.
--
Karl Williamson

p5pRT · 2016-03-01T17:06:31Z

@khwilliamson - Status changed from 'open' to 'pending release'

p5pRT · 2016-03-01T17:18:20Z

From @khwilliamson

On 03/01/2016 09:44 AM, Tom Wyant via RT wrote:

This warning is suppressed in PPIx-Regexp 0.048, which went to PAUSE yesterday. There's no test coverage tool that I know of to test regex coverage, but I believe the regex was functioning correctly.

What happened was that the lazy way to include a right square bracket in a bracketed character class is to put it first. Having square brackets on the brain, I put the left square bracket next, and followed with the POSIX class. The fix was to shuffle the \@ before the POSIX class, as suggested by perldiag. I admit that I grumbled to myself a little over this, but on reflection it's no different than shuffling that right square bracket to the front.

As I noted to a correspondent on this issue, Perl is so overloaded that it is fairly easy to write something that is valid Perl but nevertheless quite different than the author's intention, and warnings like this have a long history. But if you will allow the opinion of someone whose normal involvement with the porters' mailing list is reading (with profound thanks!) SawyerX's summaries, I personally would appreciate the suppression of the false positive (which I believe this is) if it can be done without eliminating the message completely.

With thanks to all of you for the continuous improvement of Perl 5,
Tom Wyant

---
via perlbug: queue: perl5 status: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127581

I'm sorry that you felt you had to change your code. Your code was
correct, and it was Perl that needed to change. And blead has now been
changed to fix this problem, and so your original code would now work
properly without a warning. The code the regex pattern compiled to was
always correct; it just had an inappropriate warning raised.

There will be false positives with this new warning, but there should
not be any where there is actually a legal posix class, as in your case.

p5pRT · 2016-05-13T04:04:56Z

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.

With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

p5pRT · 2016-05-13T04:04:56Z

@khwilliamson - Status changed from 'pending release' to 'resolved'

p5pRT closed this as completed May 13, 2016

p5pRT added the Severity Low label Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5.23.8 + "Assuming NOT a POSIX" causes spurious warning in PPIx::Regexp::Token::Literal #15190

5.23.8 + "Assuming NOT a POSIX" causes spurious warning in PPIx::Regexp::Token::Literal #15190

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

p5pRT commented Mar 1, 2016

p5pRT commented Mar 1, 2016

p5pRT commented Mar 1, 2016

p5pRT commented Mar 1, 2016

p5pRT commented May 13, 2016

p5pRT commented May 13, 2016

5.23.8 + "Assuming NOT a POSIX" causes spurious warning in PPIx::Regexp::Token::Literal #15190

5.23.8 + "Assuming NOT a POSIX" causes spurious warning in PPIx::Regexp::Token::Literal #15190

Comments

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

From @kentfredric

p5pRT commented Feb 21, 2016

From @tonycoz

p5pRT commented Feb 21, 2016

p5pRT commented Feb 21, 2016

From @cpansprout

p5pRT commented Feb 21, 2016

From @khwilliamson

p5pRT commented Mar 1, 2016

From @trwyant

p5pRT commented Mar 1, 2016

From @khwilliamson

p5pRT commented Mar 1, 2016

p5pRT commented Mar 1, 2016

From @khwilliamson

p5pRT commented May 13, 2016

From @khwilliamson

p5pRT commented May 13, 2016