found a weird case where MS eats compilation errors

[barefootcoder]

Fix and failing test case here: barefootcoder/method-signatures@e0cd9bc9897a486e2bb3

Basically, it looks like the fact that there's a pending error in $@ is confusing the eval inside PPI (specifically, PPI::Node's _wanted(), which is called from find_first()). Localizing $@ inside split_proto() fixes the issue.

[schwern]

On 2011.4.16 1:47 AM, barefootcoder wrote:

Fix and failing test case here: barefootcoder/method-signatures@e0cd9bc9897a486e2bb3

Basically, it looks like the fact that there's a pending error in $@ is confusing
the eval inside PPI (specifically, PPI::Node's _wanted(), which is called from
find_first()). Localizing $@ inside split_proto() fixes the issue.

Hmm, PPI::Node is vulnerable to $@ pollution all over the place. Would be
nice to have that fixed.

Good catch.

Can I cherry pick that fix into master?

E: "Would you want to maintain a 5000 line Perl program?"
d: "Why would you write a 5000 line program?"

[barefootcoder]

Good catch.

Thanx! Purely accidental that I found it, of course. I was hacking
up something quick for something else entirely and I decided to use MS
(partially to get a feel for how useful it could be outside of Moose,
and partially just to get more testing in on it), and I just happened
to create a syntax error while I was refactoring. But the error it
showed was the one from split_proto() about not being able to find a
statement, which I knew was bogus. And after that it was a matter of
just beating at it until I figured it out (debugger was useless on
this one; I ended up filling up PPI::Node with good ol' fashioned
print STDERR's).

Can I cherry pick that fix into master?

Cherrypick away, monsieur. I've tried to be clear in the issues as to
what's part of the MSM project vs what's a separate issue. Anything
outside of MSM stuff could be cherrypicked, and I'd say quite a bit
of it probably should be cherrypicked. The POD/comments only stuff,
the fix for comments inside signatures ... also possibly the change to
fix type-checking non-supplied optional params (though that one is a
slightly more pervasive change, so I'd understand if you wanted to
hold off there). All the commits should be independent and not give
you any merge conflicts, I would think. (In retrospect, I really
should have put the MSM stuff onto a separate branch, but I didn't
think of it until after I'd already started and I didn't want to stop
and figure out how to move stuff around in git. So, sorry about
that.)

[schwern]

Merged. The PPI issue is open as #28.