Paul Johnson
2013-01-02 14:36:45 UTC
In accordance with the terms of my grant from TPF this is the report for
weeks 30-32 of my work on improving Devel::Cover.
This report covers 08.12 to 28.12.
Last week I mentioned the pull requests I had received. I this week I merged
a pull request from Steffen Schwigon adding some important modules to
cpancover.com. I'm always happy to add modules that people would like to see
on cpancover, so if there's anything you think should be on there, let me
know.
And speaking of cpancover.com, you should now be able to see a note at the
foot of the title page thanking Bytemark for their generosity in donating the
server that hosts the site. See last week's report for the details. Thanks
again to Bytemark for that.
I spent some time looking at https://github.com/pjcj/Devel--Cover/issues/38.
This is a bug report from David Cantrell in which there is different behaviour
when a program is run under Devel::Cover to when it is run normally. To me
this is the most serious type of problem. Devel::Cover should not alter the
output of the program being covered. This is more important than missing or
giving incorrect coverage information, or being slow.
--------------------------------------------------------------------------------
I have narrowed the problem down to the following code:
my $attributed;
sub MODIFY_CODE_ATTRIBUTES {
my ($class, $code, @attrs) = @_;
$attributed = $code;
return;
}
my $foo = sub :MyAttribute {};
print "$attributed, $foo\n";
If you run this code normally, you find that $attributed and $foo have
the same value. This is as it should be since the docs for
attributes.pm say that $code should be a reference to the sub with the
attribute. (Actually, the docs don't explicitly say that but I think
the intent is clear.)
When run under the debugger you find that $code is actually a different
subroutine reference. The reason is this little bit of code in
pad.c:1727:
if (PL_cv_has_eval || PL_perldb) {
const CV *cv;
for (cv = PL_compcv ;cv; cv = CvOUTSIDE(cv)) {
if (cv != PL_compcv && CvCOMPILED(cv))
break; /* no need to mark already-compiled code */
if (CvANON(cv)) {
DEBUG_Xv(PerlIO_printf(Perl_debug_log,
"Pad clone on cv=0x%"UVxf"\n", PTR2UV(cv)));
CvCLONE_on(cv);
}
CvHASEVAL_on(cv);
}
}
The code is preceded by this comment:
/* If this CV has had any 'eval-capable' ops planted in it
* (ie it contains eval '...', //ee, /$var/ or /(?{..})/), Then any
* anon prototypes in the chain of CVs should be marked as cloneable,
* so that for example the eval's CV in C<< sub { eval '$x' } >> gets
* the right CvOUTSIDE.
* If running with -d, *any* sub may potentially have an eval
* executed within it.
*/
So, there are a few questions.
First, is it correct that these two subroutine refs should be identical?
If so, should that also hold when running under the debugger?
And if it shouldn't necessarily hold, is that also true for $^P being
set to any value, as is currently the case?
The reason this shows up in Devel::Cover is because $^P is set to 0x104
which, I think, doesn't meet the conditions in the comment.
--------------------------------------------------------------------------------
So the upshot of this one is that I don't think this is a bug in Devel:Cover.
It seems that the expectation that $code should be a reference to the sub with
the attribute is false. As Dave Mitchell wrote:
] Given that MODIFY_CODE_ATTRIBUTES() is only called once at compile time,
] while sub {} can be called multiple times at runtime, I'm not sure it
] makes sense in general for $attributed to match anything; e.g. in
]
] my $x;
] push @foo, sub :MyAttribute {$x} for 1..10;
]
] which (if any) of the elements of @foo would you expect to be equal to
] $attributed?
I also spent some time tracking down a problem that someone had with a
Catalyst application that was giving a Unique Constraint Error in SQLite only
when run with Devel::Cover. As it happens, that turned out to be exactly the
same problem. So that's Dancer and Catalyst where this problem is showing up.
Then I looked into the possibility of adding a link on MetaCPAN to a module's
coverage on CPANcover. I put together a patch to the front-end to see whether
there was general interest, which seems to be the case. However, some
back-end support is probably needed since CPANcover only has coverage for a
subset of CPAN, and I don't really want to be directing people to 404s. With
backend support it will also be possible to go directly to the coverage for a
specific file.
I've not made a backend patch since I'm not really sure how to do it. I'm
hoping someone will offer to either do that, or tell me how to do it. The
patch is at https://github.com/CPAN-API/metacpan-web/pull/732 should anyone
feel like taking this further.
Merged pull requests:
37 Add modules to cpancover
You can see the commits at https://github.com/pjcj/Devel--Cover/commits/master
Hours worked:
08.12 6:20
22.12 2:30
Total 8:50
weeks 30-32 of my work on improving Devel::Cover.
This report covers 08.12 to 28.12.
Last week I mentioned the pull requests I had received. I this week I merged
a pull request from Steffen Schwigon adding some important modules to
cpancover.com. I'm always happy to add modules that people would like to see
on cpancover, so if there's anything you think should be on there, let me
know.
And speaking of cpancover.com, you should now be able to see a note at the
foot of the title page thanking Bytemark for their generosity in donating the
server that hosts the site. See last week's report for the details. Thanks
again to Bytemark for that.
I spent some time looking at https://github.com/pjcj/Devel--Cover/issues/38.
This is a bug report from David Cantrell in which there is different behaviour
when a program is run under Devel::Cover to when it is run normally. To me
this is the most serious type of problem. Devel::Cover should not alter the
output of the program being covered. This is more important than missing or
giving incorrect coverage information, or being slow.
--------------------------------------------------------------------------------
I have narrowed the problem down to the following code:
my $attributed;
sub MODIFY_CODE_ATTRIBUTES {
my ($class, $code, @attrs) = @_;
$attributed = $code;
return;
}
my $foo = sub :MyAttribute {};
print "$attributed, $foo\n";
If you run this code normally, you find that $attributed and $foo have
the same value. This is as it should be since the docs for
attributes.pm say that $code should be a reference to the sub with the
attribute. (Actually, the docs don't explicitly say that but I think
the intent is clear.)
When run under the debugger you find that $code is actually a different
subroutine reference. The reason is this little bit of code in
pad.c:1727:
if (PL_cv_has_eval || PL_perldb) {
const CV *cv;
for (cv = PL_compcv ;cv; cv = CvOUTSIDE(cv)) {
if (cv != PL_compcv && CvCOMPILED(cv))
break; /* no need to mark already-compiled code */
if (CvANON(cv)) {
DEBUG_Xv(PerlIO_printf(Perl_debug_log,
"Pad clone on cv=0x%"UVxf"\n", PTR2UV(cv)));
CvCLONE_on(cv);
}
CvHASEVAL_on(cv);
}
}
The code is preceded by this comment:
/* If this CV has had any 'eval-capable' ops planted in it
* (ie it contains eval '...', //ee, /$var/ or /(?{..})/), Then any
* anon prototypes in the chain of CVs should be marked as cloneable,
* so that for example the eval's CV in C<< sub { eval '$x' } >> gets
* the right CvOUTSIDE.
* If running with -d, *any* sub may potentially have an eval
* executed within it.
*/
So, there are a few questions.
First, is it correct that these two subroutine refs should be identical?
If so, should that also hold when running under the debugger?
And if it shouldn't necessarily hold, is that also true for $^P being
set to any value, as is currently the case?
The reason this shows up in Devel::Cover is because $^P is set to 0x104
which, I think, doesn't meet the conditions in the comment.
--------------------------------------------------------------------------------
So the upshot of this one is that I don't think this is a bug in Devel:Cover.
It seems that the expectation that $code should be a reference to the sub with
the attribute is false. As Dave Mitchell wrote:
] Given that MODIFY_CODE_ATTRIBUTES() is only called once at compile time,
] while sub {} can be called multiple times at runtime, I'm not sure it
] makes sense in general for $attributed to match anything; e.g. in
]
] my $x;
] push @foo, sub :MyAttribute {$x} for 1..10;
]
] which (if any) of the elements of @foo would you expect to be equal to
] $attributed?
I also spent some time tracking down a problem that someone had with a
Catalyst application that was giving a Unique Constraint Error in SQLite only
when run with Devel::Cover. As it happens, that turned out to be exactly the
same problem. So that's Dancer and Catalyst where this problem is showing up.
Then I looked into the possibility of adding a link on MetaCPAN to a module's
coverage on CPANcover. I put together a patch to the front-end to see whether
there was general interest, which seems to be the case. However, some
back-end support is probably needed since CPANcover only has coverage for a
subset of CPAN, and I don't really want to be directing people to 404s. With
backend support it will also be possible to go directly to the coverage for a
specific file.
I've not made a backend patch since I'm not really sure how to do it. I'm
hoping someone will offer to either do that, or tell me how to do it. The
patch is at https://github.com/CPAN-API/metacpan-web/pull/732 should anyone
feel like taking this further.
Merged pull requests:
37 Add modules to cpancover
You can see the commits at https://github.com/pjcj/Devel--Cover/commits/master
Hours worked:
08.12 6:20
22.12 2:30
Total 8:50
--
Paul Johnson - ***@pjcj.net
http://www.pjcj.net
Paul Johnson - ***@pjcj.net
http://www.pjcj.net