Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Ovid (2709)

Ovid
  (email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)

Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.

Journal of Ovid (2709)

Tuesday February 26, 2008
04:36 AM

Here's The Bug

[ #35763 ]

Yesterday's Find the Bug was pretty straightforward for folks, but I'll lay it out here. Basically, this causes the problem:

sub foo {
    while ($_ = @_) {}
}

foreach ('bar') { foo($_) }

That fails with a "Modification of a read-only value" error. This is because foreach without a declared variable implicitly creates an alias to each element in the list, in this case, a string constant. As a result, $_ has a value when foo() is called and you find yourself attempting to assign to a constant.

I've been bitten by this before and I know some of you have, so you might be asking yourself an obvious question. Why did I write "it took me a while to see [the bug]?"

Because the bug was triggered by this:

use Moose;

extends 'Some::Class';

And somehow, deep in the bowels of the Moose, it fails to load Some::Class because that class needs some other class which requires Exporter::NoWork and the string constant Some::Class gets set as the value of $_ and Exporter::NoWork has a while ($_ = shift) line in it. The first fix was strange:

use Moose;

use Some::Class;
extends 'Some::Class';

The real fix is a patch to Exporter::NoWork (though I suspect Moose is being a touch naughty, too), but it turns out that this module only has one test, and it's a load test. I hate sending patches to modules with no test suite :(

Once again, the moral of today's story: if you use anything which potentially impacts global state, please try to limit it to your local scope!

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • Well, Moose is not really doing anything very nasty here, the class loading code is quite sane (we "borrowed" it from Class::Inspector). But we are looping through @_ using $_ when we load those classes, so that is where the issue comes from.

    I have adjusted all code that comes in contact with the class loader so that none of it uses $_ anymore. This should eliminate the issue you are seeing with the Exporter::NoWork and modules like it. This will be available in the next release of Class::MOP/Moose.

    -

    • I don't think it's terribly nasty, but the implicit aliasing can cause strange behavior if other modules don't play well with that. Imagine this:

      foreach (@_) {
          some_other_function($_);
          # do something with $_
      }

      If some_other_function did something silly like s/(.*)/scalar reverse $1/e, then the above code would break. Guarding against side-effects is a good thing. Of course, since @_ contains aliases, I suppose that the way to be truly paranoid would be:

      foreach my $

  • Having just got your bug reports, I'm in the middle of writing a test suite now... I wrote E::NW before I'd really cottoned on to the value of testing, and haven't touched it since. I've already found another latent bug.

    Unfortunately, local $_ isn't safe either. If $_ is aliased to a member of a tied hash or array, it isn't localised properly and the value in the hash is affected as well:

    tie my %h, 'Tie::StdHash';
    $h{a} = 1;
    for ($h{a}) {
        local $_ = 2;
        print $h{a};    # 2

    • Oh, nice catch. We've got local workarounds in place, but I'm happy to know that you've spotted more than I did :)