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!
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.
- Stevan
Re:Re; Moose
Ovid on 2008-02-26T16:21:33
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 $var (@_ = @_) {
some_other_function(my $temp = $var);
# do something with $var
}After a while, too much paranoia on my part would get ridiculous, but I'm now understanding better why Larry Wall rejected my proposal to add an 'is rw' shortcut for sub/method signatures in Perl 6 (right now you have to declare this for every argument and that gets tedious, but it's the right thing to do).
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:
If you call external code in the scope of the change (as E::NW does) this could be a problem. The workarounds are either to localise all oftie my %h, 'Tie::StdHash';
$h{a} = 1;
for ($h{a}) {
local $_ = 2;
print $h{a}; # 2, but should be 1
}
print $h{a}; # back to 1 again
*_ which, besides being ugly, clobbers @_ as well, or to use for instead of assigning to $_ at all. for doesn't suffer from this problem, and neither does 5.10's my $_.
Re:no test suite
Ovid on 2008-02-26T20:43:21
Oh, nice catch. We've got local workarounds in place, but I'm happy to know that you've spotted more than I did
:)