Changes in m//m in 5.10

[Andy Lester]

Can someone explain why this is so?

$ cat test.pl
my $buffer = qq{blah\nfoo\nblah};
my $regex = qr/^foo/;
print "Running $], ", $buffer =~ /$regex/m ? 'found' : 'not found',
"\n";

$ perl test.pl
Running 5.008008, found

$ perl510 test.pl
Running 5.010000, not found

ack fails its tests because of this behavior, which is what led me
down this path.

xoxo,
Andy

--
Andy Lester => an...@petdance.com => www.petdance.com => AIM:petdance

[Rafael Garcia-Suarez]

On 19/12/2007, Andy Lester <an...@petdance.com> wrote:
> Can someone explain why this is so?
>
> $ cat test.pl
> my $buffer = qq{blah\nfoo\nblah};
> my $regex = qr/^foo/;
> print "Running $], ", $buffer =~ /$regex/m ? 'found' : 'not found',
> "\n";
>
> $ perl test.pl
> Running 5.008008, found
>
> $ perl510 test.pl
> Running 5.010000, not found
>
> ack fails its tests because of this behavior, which is what led me
> down this path.

The effect of /m is no longer global, but it now only affects the
regexp it was applied to. No more unwanted side effects! See bug
http://rt.perl.org/rt3/Ticket/Display.html?id=22354
Incidentally, that was to fix this bug that $* was removed.

[Andy Lester]

On Dec 19, 2007, at 3:26 PM, Rafael Garcia-Suarez wrote:

> The effect of /m is no longer global, but it now only affects the
> regexp it was applied to. No more unwanted side effects! See bug
> http://rt.perl.org/rt3/Ticket/Display.html?id=22354
> Incidentally, that was to fix this bug that $* was removed.

So really, all this time the /m should have gone on the qr//, not on
the m/$regex/ ?

xoa

[Ronald J Kimball]

But in Andy's example, it is not affecting the regex it was applied to.

That bug report was for /m propagating to a regex used in a subroutine
called from the replacement part of a substitution, which seems to be a
completely different situation.

Ronald

[Demerphq]

On 19/12/2007, Andy Lester <an...@petdance.com> wrote:
>

> On Dec 19, 2007, at 3:26 PM, Rafael Garcia-Suarez wrote:
>
> > The effect of /m is no longer global, but it now only affects the
> > regexp it was applied to. No more unwanted side effects! See bug
> > http://rt.perl.org/rt3/Ticket/Display.html?id=22354
> > Incidentally, that was to fix this bug that $* was removed.
>
>
> So really, all this time the /m should have gone on the qr//, not on
> the m/$regex/ ?

Yes. Although it wasnt clear to me straight away. :-)

Although, IMO its an arguable case slightly in this case due to there
not being anything else in the pattern. If there was anything else
involved in the pattern then the situation would be clear that 5.10 is
correct. With the bare pattern however its arguable that we could
check the modifiers on the m// and see if they match that of $regex
and recompile if not. This would be DWIM but would also negate most of
the point of using qr//.

Probably it would be better to simply warn "Useless use of modifiers
on m/// at line ...".

cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

[Andy Lester]

On Dec 19, 2007, at 4:10 PM, demerphq wrote:

> Probably it would be better to simply warn "Useless use of modifiers
> on m/// at line ...".

Yes. It would have saved me a lot of digging.

This change blows my entire way of thinking about qr//. I need
recalibration.

xoxo,
Andy

[Eric Brine]

>
> That bug report was for /m propagating to a regex used in a subroutine
> called from the replacement part of a substitution, which seems to be a
> completely different situation.
>

Note that qr/^foo/ instructs Perl to turn off multi-line mode

>perl -le"print qr/^foo/;"
(?-xism:^foo)

So it's the same situation. Multi-line mode is being propagated to other
regexes with multi-line mode off, solely because the "victim" regexes where
executed while processing a multi-line regex.

ELB

[A. Pagaltzis]

* demerphq <deme...@gmail.com> [2007-12-19 23:15]:

> With the bare pattern however its arguable that we could check
> the modifiers on the m// and see if they match that of $regex
> and recompile if not. This would be DWIM but would also negate
> most of the point of using qr//.

I feel a vague unease about piling ever more exceptional cases
onto the rules… Does this really give the user something that is
a) needed b) hard to achieve otherwise?

> Probably it would be better to simply warn "Useless use of
> modifiers on m/// at line ...".

+1

That is much more agreeable.

Regards,
--
Aristotle Pagaltzis // <http://plasmasturm.org/>

[ava...@gmail.com]

On 12/19/07, demerphq <deme...@gmail.com> wrote:
> Probably it would be better to simply warn "Useless use of modifiers
> on m/// at line ...".

Here's an incomplete patch that implements this, could someone with
better luser communication skills fix the diagnostics message please?

[Andy Lester]

On Dec 20, 2007, at 5:25 PM, Ævar Arnfjörð Bjarmason wrote:

>> Here's an incomplete patch that implements this, could someone with
>> better luser communication skills fix the diagnostics message please?
>

> Here's a complete one, thanks dmq!

And here's me scolding myself for not trying ack with the RCs so this
could have gotten in the 5.10.0 release. :-(

[Andy Armstrong]

On 20 Dec 2007, at 23:31, Andy Lester wrote:
> And here's me scolding myself for not trying ack with the RCs so
> this could have gotten in the 5.10.0 release. :-(

While we're doing mea-culpas - I noticed it was broken and forgot to
tell you...

--
Andy Armstrong, Hexten

[Rafael Garcia-Suarez]

On 21/12/2007, Ævar Arnfjörð Bjarmason <ava...@gmail.com> wrote:

> Here's a complete one, thanks dmq!

I haven't tested, but isn't this going to warn even in cases like the
following?:
my $r = qr/^foo/;
/$r|^bar/m;

(moreover, where are the tests ? :)

[Demerphq]

No it wont actually. The warning would only be raised when the $qr
variable is the only thing in the pattern. The diagnostics should read
(added an 'only'):

+(W regexp) Your pattern contains only a variable containing a precompiled
+regular expression pattern whose behavior will not be changed by the
+pattern modifiers provided.

$qr=/^foo/;
$x=~/$qr/m; # warns;
$x=~/$qr|foo/m; #does not warn
$x=~/$qr/x; # does not warn (/x doesnt change match semantics)

[Andy Lester]

FWIW, I wrote a little article about all this:

http://perlbuzz.com/mechanix/2007/12/code-broken-by-regex-fixes-in.html

xoxo,
Andy

[ava...@gmail.com]

On Dec 21, 2007 12:08 PM, demerphq <deme...@gmail.com> wrote:
> No it wont actually. The warning would only be raised when the $qr
> variable is the only thing in the pattern. The diagnostics should read
> (added an 'only'):
>
> +(W regexp) Your pattern contains only a variable containing a precompiled
> +regular expression pattern whose behavior will not be changed by the
> +pattern modifiers provided.
>
> $qr=/^foo/;
> $x=~/$qr/m; # warns;
> $x=~/$qr|foo/m; #does not warn
> $x=~/$qr/x; # does not warn (/x doesnt change match semantics)

In the current implementation yes, but we might potentially want to
warn in the future on useless modifiers in general, eg:

$x =~ /(?-msix:^foo)/msi;

[Joshua ben Jore]

That would be hilarious as it'd presumably throw warnings at people
who uselessly used the /msx modifiers as suggested by Damian's PBP.
Mostly I find uselessly adding /msx makes it harder to read code
because I have to guess more about intent and it's noise. I'd be all
for this sort of warning.

Josh

[Andy Lester]

On Dec 22, 2007, at 5:07 PM, Joshua ben Jore wrote:

> That would be hilarious as it'd presumably throw warnings at people
> who uselessly used the /msx modifiers as suggested by Damian's PBP.
> Mostly I find uselessly adding /msx makes it harder to read code
> because I have to guess more about intent and it's noise. I'd be all
> for this sort of warning.

We're having just this discussion on the perlcritic mailing list right
now.

Personally, I have no problem with warning about the useless /sm.
It's useful to tell someone "you're doing something useless", just
like an expression in void context.

The reflexive /sm is like backslashing every non-alpha character in a
regex because the author can't remember what's a metacharacter and
what isn't, so he writes things like qr/Bang\-Bang\!/ because he
thinks that - and ! might maybe possibly be meta.

[Chris Dolan]

I would like to make a few points in opposition to this new warning.
Before I do so, I'd like to acknowledge that such a warning would
certainly be useful in alerting 5.8 -> 5.10 upgraders about the bug
fix. And I appreciate the effort expended in minimizing the scope of
the warning. But I think it's a readability loss overall, and when
5.8 is passé, we'll still be stuck with it.

1) The Perl Best Practices book recommends putting /m on all
regexes for consistency and readability. In that paradigm, '^' and
'$' always mean start/end-of-line and never start/end-of-string,
which lightens the reader's mental load.
2) Perl::Critic has been recommending /m on all regexes since 0.13,
so lots of people have adopted this habit.
3) People who adopted that habit were not bitten by this 5.8 ->
5.10 behavior change.
4) The new warning would mean that the above practice is now
officially discouraged, at least in terms of the absoluteness of the
practice.
5) Authors (like me) with working 5.8 code will have to remove
harmless /m modifiers from various places just to get their code to
run quietly.
6) While the modifier is indeed useless in 5.10, it's not at all
harmful or wasteful, unlike say sort in void context which wasted CPU.

If this patch does go in, we'll probably adapt Perl::Critic to know
about this special case, and I'll probably go through all of my CPAN
code to remove the extraneous modifiers, and I'll forget about this
whole issue in a few months. But what a pain for so little gain...

Chris

[Andy Lester]

On Dec 22, 2007, at 6:13 PM, Chris Dolan wrote:
> 2) Perl::Critic has been recommending /m on all regexes since 0.13,
> so lots of people have adopted this habit.

Perl::Critic users, and indeed PBP readers, is a very small subset of
Perl users.

My only concern is the user who does this:

my $re = qr/something/;
if ( $str =~ /$re/m )...

and has no idea why it doesn't work. That user is the one we must pay
the most attention to. If there's some way to let him know that his
invocation of the regex is wrong, I'd love to hear it.

I can't imagine that I'm the only one out there in the universe of
Perl users who's been writing regexes like that incorrectly,
accidentally taking advantage of this now-fixed bug. If I'd had a
warning like that since I started using regex objects, I'd never have
gotten into the bad habit.

[Demerphq]

On 23/12/2007, demerphq <deme...@gmail.com> wrote:

> I dont see the problem. If you use the same modifiers everywhere there
> will be no warning.
>
> The only place it will warn is when there is a mismatch between the
> modifiers on a qr// and on the match processing the qr//.
>
> IOW:
>
> $qr=~/^foo/;
> if (/$qr/m) {}
>
> If you never do stuff like this, for instance if you always write:
>
> $qr=~/^foo/m;
> if (/$qr/m) {}
>
> there will be no warning.

BTW, i think that there will be better solutions to PBP type issues
coming soon. In particular its in the plans to be able to set default
modifiers on patterns. So to be PBP compliant youd say something like

use re::modifiers 'msx';

cheers,
yves

[Demerphq]

On 23/12/2007, Chris Dolan <ch...@chrisdolan.net> wrote:

I dont see the problem. If you use the same modifiers everywhere there
will be no warning.

The only place it will warn is when there is a mismatch between the
modifiers on a qr// and on the match processing the qr//.

IOW:

$qr=~/^foo/;
if (/$qr/m) {}

If you never do stuff like this, for instance if you always write:

$qr=~/^foo/m;
if (/$qr/m) {}

there will be no warning.

--

[Nicholas Clark]

On Sun, Dec 23, 2007 at 12:20:47PM +0100, demerphq wrote:
> On 23/12/2007, Chris Dolan <ch...@chrisdolan.net> wrote:

> > If this patch does go in, we'll probably adapt Perl::Critic to know
> > about this special case, and I'll probably go through all of my CPAN
> > code to remove the extraneous modifiers, and I'll forget about this
> > whole issue in a few months. But what a pain for so little gain...
>
> I dont see the problem. If you use the same modifiers everywhere there
> will be no warning.

This was what I was thinking too. But I wasn't sure sure, so I didn't e-mail.

> The only place it will warn is when there is a mismatch between the
> modifiers on a qr// and on the match processing the qr//.
>
> IOW:
>
> $qr=~/^foo/;
> if (/$qr/m) {}
>
> If you never do stuff like this, for instance if you always write:
>
> $qr=~/^foo/m;
> if (/$qr/m) {}
>
> there will be no warning.

This check would work at runtime, so if $qr changes value it will only warn on
a flag mismatch?

Nicholas Clark

[Demerphq]

It will warn IFF the only content in the pattern is a $qr and the
modifiers the qr// was compiled with are different from the modifiers
on the actual match such that the discrepancy would change the
semantics of the match.

So assuming that Chris always uses /m the only time that Chris might
see warnings is if he wrote a procedure that accepts qr// objects from
outside his code and then executed them using the /m modifier. But
then he could disable the warning, or drop the /m modifier, or
whatever.

I really cant see this warning causing a lot of trouble.

yves

[Dr.Ruud]

demerphq schreef:

> $qr=/^foo/;

ITYM: $qr = qr/^foo/;

Or does /.../ imply qr in perl 5.10?
(wouldn't be bad)

> $x=~/$qr/m; # warns;

Or does this also warn if $qr isn't a compiled regex?

> $x=~/$qr|foo/m; #does not warn
> $x=~/$qr/x; # does not warn (/x doesnt change match semantics)

That is all quite hard to read with so little whitespace.

--
Affijn, Ruud

"Gewoon is een tijger."

[av...@cpan.org]

Here's the patch with tests:

[Chris Dolan]

On Dec 23, 2007, at 5:51 AM, demerphq wrote:

> It will warn IFF the only content in the pattern is a $qr and the
> modifiers the qr// was compiled with are different from the modifiers
> on the actual match such that the discrepancy would change the
> semantics of the match.
>
> So assuming that Chris always uses /m the only time that Chris might
> see warnings is if he wrote a procedure that accepts qr// objects from
> outside his code and then executed them using the /m modifier. But
> then he could disable the warning, or drop the /m modifier, or
> whatever.
>
> I really cant see this warning causing a lot of trouble.
>
> yves
>

Oh, if my practice of /m everywhere doesn't trigger the warning, then
I have no problem at all. Without reading the patch I had assumed it
was a compile-time warning that looked for a single scalar in the m//
m. My mistake.

Keep up the good work! :-)

Chris

[Abigail]

As an avid constructor of regexes out of small parts, I really, really
hope this warning doesn't make it. It would mean program needs more code
just to avoid warnings.

IMO, warnings should be useful, that is, they should warn the programmer
when something unusual is happening. But warnings cease to be useful if
the trigger wrongly. One false warning is worse than a dozen of 'missed'
warnings.

Giving a warning just because the programmer could have saved a keystroke
is, IMO, a really bad idea.

Abigail

[ava...@gmail.com]

On Dec 23, 2007 11:20 AM, demerphq <deme...@gmail.com> wrote:
> I dont see the problem. If you use the same modifiers everywhere there
> will be no warning.

Actually the current implementation warns about useless use of
modifers. It could be changed so that it doesn't warn when your
useless modifiers are equal to that of the compiled pattern but that's
not what the current implementation does.

[av...@cpan.org]

abi...@abigail.be (Abigail) writes:

> IMO, warnings should be useful, that is, they should warn the programmer
> when something unusual is happening. But warnings cease to be useful if
> the trigger wrongly. One false warning is worse than a dozen of 'missed'
> warnings.

The real problem here is that warnings.pm is doing a lot of different
things for a lot of different people. In some cases it's warning for
things that are almost definitely logic errors introduced by the
programmer (C<@s =~ /123/>), some that are bad practices (C<undef()
. "foo">) and some are things that are prone to be misunderstood (C<qw(#
comment?)>).

The problem with this arrangement is that we have to tiptoe a line
between providing useful warnings and annoying people with our nits that
they may not care about. I find myself more in the latter category
already which is why I don't use warnings in my code[1].

> Giving a warning just because the programmer could have saved a keystroke
> is, IMO, a really bad idea.

Often while people introduce redundancy they do so because they don't
fully understand what they're doing. In this case because they're likely
to think C</$qr_object/i> changes their match semantics while in fact it
does not.

Footnotes:

1. I use GNU Emacs and its flymake mode continually runs perl -c on perl
the code in my buffer when I'm editing it. When I use warnings I find
that my code as well as things I have to maintain lights up like a
Christmas tree with warning highlights although it's written just the
way I want it.

I suppose I could extend my editor to change "use warnings;" to "use
warnings qw(whitelist of things that don't annoy me);" to get around
this, but that's a project for another day.

[Demerphq]

Ah. then we aught to change that. It should only warn when the
modifers would have resulted in different match semantics.

Yves

[Eric Brine]

On Wed, Dec 19, 2007 at 5:26 PM, Rafael Garcia-Suarez
<rgarci...@gmail.com> wrote:
> On 19/12/2007, Andy Lester <an...@petdance.com> wrote:
>> Can someone explain why this is so?
>>
>> $ cat test.pl
>> my $buffer = qq{blah\nfoo\nblah};
>> my $regex = qr/^foo/;
>> print "Running $], ", $buffer =~ /$regex/m ? 'found' : 'not found',
>> "\n";
>>
>> $ perl test.pl
>> Running 5.008008, found
>>
>> $ perl510 test.pl
>> Running 5.010000, not found
>>
>> ack fails its tests because of this behavior, which is what led me
>> down this path.
>
> The effect of /m is no longer global, but it now only affects the
> regexp it was applied to. No more unwanted side effects! See bug
> http://rt.perl.org/rt3/Ticket/Display.html?id=22354
> Incidentally, that was to fix this bug that $* was removed.

On a related note,

perlop claims "If a precompiled pattern is embedded in a larger
pattern then the effect of 'msixp' will be propagated appropriately."
It needs to be fixed.

Eric Brine

[Demerphq]

2008/6/18 Eric Brine <ike...@adaelis.com>:

What needs to be fixed? The docs? Or the behaviour?

IMO the behaviour is right (now) and if anything should change its the docs.