On Fri, Jun 25, 2004 at 11:07:06AM -0400, Geoffrey Young wrote:

> hi paul :)
> 
> I recently discovered an issue with nested subroutines while using
> Devel::Cover with Parse::Yapp.  the basic issue is that some subroutines are
> not discovered by Devel::Cover and thus no metrics are generated.
> 
> there are two files in the tarball.  Foo.pm is a minimal test case showing
> that two subroutines are missing (one defined and one anonymous).  Parser.pm
> is an autogenerated file based on Parser.yp that illustrates the same thing
> - note that the _missing() subroutine is missing from coverage results (as
> well as a handful of anonymous nested subroutines I think).
> 
> the only interesting thing is that the behavior of my minimal test case and
> Parser.pm seem to be opposite - in the former the missing subroutine is the
> one that contains the anonymous sub, while in the latter is is the named
> subroutine that occurs after a nested anonymous sub.
> 
> anyway, let me know if there is anything else I can do - I realize this is
> kind of obscure, so no pressure :)
> 
> oh, and I tested using 0.46 and both 5.8.4 and 5.8.0.

Thanks a lot for the test cases.  I think there are two separate bugs
here, but I'm only going to take responsibility for one ;-)

First, mine.  The problem with Foo.pm (the minimal test case) is that
completely empty subroutines (that is subs which contain no statements
at all) are ignored as far as subroutine coverage is concerned.  That is
the case for both named and anonymous subs.  The op tree for an empty
sub didn't contain the structure I was looking for, so it wasn't
recognised as a sub.

I have put in a fix for this, but it only works with Perl 5.8.2 and
later versions.  I've not gone trying to get it to work with earlier
versions, since it is pretty obscure and I prefer to keep the code
reasonably clean.  Or maybe I'm just too lazy.  In any case, let me know
if this is going to cause anyone problems.  I have documented it as a
bug though, and upped the recommended version to use from 5.8.1 to 5.8.2.

Then the second bug.  The problem here is that if you lie to perl it
will bite your bottom ;-)

Parse::Yapp appears to take portions of the grammar in Parser.yp and
insert them into Parser.pm.  In doing so, it quite reasonably wants to
reference the original grammar which means that perl will report errors
in the grammar where they appear in the grammar.  Parse::Yapp does this
by using the #line directive.  This is all well and good, and
Devel::Cover will honour that by reporting the coverage in the original
grammar.  But ...

This means that:

  1.  The coverage will not be reported in the Parser.pm module.

  2.  Devel::Cover needs to be able to find Parser.yp.  In the example
      the filename given is Parser.yp, but the file is actually at
      lib/My/Parser.yp and so Devel::Cover can't find it.  Changing the
      example to give the full filename, or putting in a link from the
      current directory fixes the problem.  I'm not sure if this is
      actually a problem with Parse::Yapp or just a result of the way
      you packaged up the testcase.

  3.  Parse::Yapp doesn't clean up after itself by setting the line
      number and filename back to what it actually is, which means that
      subsequent code coverage is reported on in the grammar file even
      though it didn't come from there, which can be somewhat
      surprising.

So I'm afraid there's not much I can do about this one - it will need to
be fed to the author of Parse::Yapp and he can decide if he wants to do
anything about it.

In any case, the first fix will be in the next release, and thanks again
for the great test cases.

-- 
Paul Johnson - [EMAIL PROTECTED]
http://www.pjcj.net

Reply via email to