Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perl 5.10 #18

Closed
barefootcoder opened this issue Apr 20, 2011 · 7 comments
Closed

Perl 5.10 #18

barefootcoder opened this issue Apr 20, 2011 · 7 comments

Comments

@barefootcoder
Copy link
Contributor

Boy, it's amazing what you can uncover running your code on different machines ...

Okay, remember when I said that method modifiers (only) had to have a "sub " inserted in front of the function body? I handled that by adding a small wrapper around inject_if_block() into MSM. However, I'm seeing a failure on Perl 5.10.0 even on the method keyword in MXD. The weird thing is, it doesn't appear to happen on every "method" ... so far just one. I can't tell what's different about that particular method yet, but I've verified that moving that inject_if_block() wrapper from MSM to MS fixes it. I also backported that to Perl 5.12, where it doesn't seem to change anything there. So it's theoretically not a bad change to make in general. However, since it's changing the way MS operates for everything, everywhere, I thought I'd better run this by you first.

I'm also seeing some minor test failures in 5.10: some error messages are slightly different than expected, and the new test for localizing $@ in the parser is failing. I don't think these are any big deal, but obviously they'll throw up bad reports on CPAN Testers, so I suppose they have to be addressed at some point. I'm just not sure I'm going to have a good opportunity to spend any time on it any time soon.

@barefootcoder
Copy link
Contributor Author

Okay, original comment about inject_if_block() still stands.

For the other bits, the error messages was a non-issue; I was doing some stupid stuff (or else I fixed it by accident). However, the failure in t/error_interruption.t looks to be problematic.

Basically, in Perl 5.12, localzing $@ in MSP is plenty sufficient. However, in Perl 5.10, it isn't. I'm not entirely sure why, either. Testing with the absolute most basic line:

perl -Ilib -le 'use lib "t/lib"; use BarfyDie'

5.12.3 gives me:

Global symbol "$foo" requires explicit package name at t/lib/BarfyDie.pm line 13.
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

whereas 5.10.0 gives me:

BEGIN not safe after errors--compilation aborted at t/lib/BarfyDie.pm line 21.
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

In 5.12, adding -d to that line just blows up immediately. In 5.10, I can get into the debugger, and I set a global watchpoint on $@, and, basically, the error is there before I localize, it comes back properly after we exit split_proto(), then disappears again when we require BeginLift (I'm assuming that require does an implicit localization of $@), and that's it. It's just gone. So I'm not sure what's going on.

And I'm unsure what the best way to fix it is. I suppose that, instead of localzing $@, we could stop and rethrow right there ... I suppose there's not much point to continuing if you know there's a compilation error at that point. But I'm not sure I can imagine all the possible layers of eval that could theoretically be going on, and I dunno if that might screw something up.

Thoughts?

@barefootcoder
Copy link
Contributor Author

Went ahead and fixed the inject_if_block problem. Other problem still stands.

barefootcoder/method-signatures@4e03081b981124d9445c

schwern added a commit that referenced this issue Jul 4, 2011
…he proper error from the

require.  Instead it just says "BEGIN not safe after errors--compilation aborted".  I presume
5.8 doesn't work either, haven't checked.

For #18
@schwern
Copy link
Contributor

schwern commented Jul 4, 2011

I made t/error_interruption.t TODO for 5.10.0 and back just so it doesn't block installation.

@barefootcoder
Copy link
Contributor Author

I'm not seeing any test failures on 5.10 any more. (I am seeing 2 TODO tests passed, but I'm not sure how much you really care about that.) You could probably close this issue.

@schwern
Copy link
Contributor

schwern commented Jul 16, 2011

2 TODO tests passed... or TODO test #2 passed?

Here's what I see with 5.10.0.

t/syntax_errors.t            (Wstat: 0 Tests: 3 Failed: 0)
  TODO passed:   2

I can't get #2 to fail on 5.10.0, 5.8.9 or 5.12.2. It seems we fixed that at some point. Also I'm not sure why that test is expecting to see an error from splitting the prototype. There's nothing invalid about it, is there?

@barefootcoder
Copy link
Contributor Author

It says:

TODO passed: 2

I always assumed that meant 2 todo tests, but perhaps I'm wrong.

Either way, it looks like 5.10 is good. I'm more concerned about 5.8, where I'm getting several failures. I'll post in that issue once I get a better idea what's going on.

@barefootcoder
Copy link
Contributor Author

No failures on 5.10 any more. I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants