Code broken by regex fixes in 5.10.0, or, why it's good to help test release candidates

My baby, ack, broke under Perl 5.10.0, because of a fix in regex behavior that I had been using unknowingly. See, I had always used my regex objects like this:

my $re = qr/^blah blah/;
if ( $string =~ /$re/sm )...

when I should have been using it like this:

my $re = qr/^blah blah/sm;
if ( $string =~ /$re/ )...

The bug in 5.8.x is that the /$re/sm would incorrectly apply the /sm modifiers to $re. This made the code happen to work, but for the wrong reason. What was especially tricky about finding my bug was that in 5.10.0, the call to /$re/sm ignores the /sm, but doesn't tell you that.

After some back and forth on p5p, a patch was submitted that gave the warning about the ignored /sm flags, but alas, Perl 5.10 was already out. It wouldn't have been so bad if it hadn't been the day AFTER it was released.

So, lesson learned: Test your code against new release candidates of Perl, both for your code's sake, AND for Perl's sake.

And y'know, now that I think of it, this is probably a great policy for Perl::Critic just waiting to happen. I wonder how many other people are doing their regexes the wrong way, too.

Categories:

5 Comments

Matt Kraai said:

If perl warns about this construction, what's the advantage of making a Perl::Critic policy for it as well? So that it can be discovered even by people not running 5.10.0?

Matt Kraai said:

Er, 5.10.1 and later, I should have said. Sorry.

Chris Dolan said:

Perl::Critic already has such a policy. It's called RegularExpressions::RequireLineBoundaryMatching and has been present in P::C since 0.13. However, an oversight caused it to apply only to m// and s/// and not qr// until this past September when I fixed it during the course of my TPF grant work.

Andy Lester said:

There are two problems.

* not applying the /m to the qr//
* applying the /m to the m//.

Your policy only handles the first case, which I'm glad it does, but is only half the battle. I just had someone else tell me that he had been doing regexes my way as well, so I'm not alone in this goof.

Tom Wyant said:

You're certainly not alone. I _did_ test Astro::SpaceTrack under 5.10.0, and _still_ had to patch.

In this case the test suite was inadequate. Still is, since I'm reluctant to look too hard at downloaded data (see Net::Dict or Astro::SIMBAD for why not), and I can't just download and bundle a file to munch on without getting permission from NORAD.

I'm not sure Test::Coverage would have helped, since I don't know how it knows the results of execution were actually looked at.

Bottom line: adequate testing is as difficult as it is essential.

Leave a comment

About this Entry

This page contains a single entry by Andy Lester published on December 20, 2007 10:52 PM.

There is no zeroth century was the previous entry in this blog.

Make vim support Perl 5.10 is the next entry in this blog.

Find recent content on the main index or look in the archives to find all content.

Other Perl Sites

Other Swell Blogs

  • geek2geek: An ongoing analysis of how geeks communicate, how we fail and how to fix it.