On Thu, Sep 30, 2010 at 16:19, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Sep 30, 2010 at 01:43:06PM +0000, Ãvar ArnfjÃrà Bjarmason wrote:
>
>> Change the regex fragment in extract_valid_address to use the qr//
>> syntax for compiled regexes, and when they're used add a /o flag so
>> they're only compiled once for the lifetime of the program.
>> [...]
>> Âsub extract_valid_address {
>> Â Â Â my $address = shift;
>> - Â Â my $local_part_regexp = '[^<>"\s@]+';
>> - Â Â my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+';
>> + Â Â my $local_part_regexp = qr/[^<>"\s@]+/;
>> + Â Â my $domain_regexp = qr/[^.<>"\s@]+(?:\.[^.<>"\s@]+)+/;
>>
>
> Hmm. But these are lexical variables, so won't we recompile them each
> time we enter the subroutine? I don't think it affects correctness, as
> this "/o":
>
>> + Â Â return $address if ($address =~ /^($local_part_regexp)$/o);
>
> means that we will compile and use the value from the first time we run
> the function.
>
> But we are unnecessarily compiling the sub-regexes each time. Not that
> this is probably a performance critical piece of code, but your "/o" is
> doing very little, and this is exactly the sort perl wankery that I find
> interesting.
IIRC different perl versions treat this differently. In more recent
versions doing:
perl -Mre=debug -E 'sub x { my $x = qr/foo/; my $y = qr/bar/;
/$x$y/ } x($_) for 1..2'
Will only compile all of those regexes once, since perl can see that
they're constant. So the /o does nothing.
We might want to keep it for self-documentation purposes to indicate
that the $local_part_regexp would never change, but it's probably best
to drop that /o altogether.
> Sadly, there is no real perl equivalent of C static local variables,
> which is what you really want. ÂUsually I would do:
>
> Â{
> Â Âmy $local_part_regexp = qr/.../;
> Â Âsub extract_valid_address {
> Â Â Â...
> Â Â}
> Â}
>
> but beware of the execution order. That works well in a module, where
> the module code is executed before anybody calls the function. But it
> breaks in something like this:
>
> Âfoo();
> Â{
> Â Âmy $foo_static_local = 5;
> Â Âsub foo {
> Â Â Âprint "$foo_static_local\n";
> Â Â}
> Â}
>
> I think you could get by with:
>
> Â{
> Â Âmy $local_part_regexp;
> Â Âsub extract_valid_address {
> Â Â Â$local_part_regexp ||= qr/.../;
> Â Â Â...
> Â Â}
> Â}
Perl has static variables like that, just not in the ancient version
we're pinned to:
sub foo {
state $foo = qr/$_[0]/
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html