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

various fixes - also cperl #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

various fixes - also cperl #5

wants to merge 3 commits into from

Conversation

rurban
Copy link

@rurban rurban commented Dec 11, 2016

fix wrong t/50reentrant_goto_sigsegv.t

wrong test count.
wrong name check. install_accessor_with_shim installs a package prefix.

XS: replace op_spare with op_typechecked

in cperl. cperl has no spare bit. we can use that one.
the called function is also enterxssub, the faster XS variant,
not entersub.

fix todo_skip syntax in t/50reentrant_goto_sigsegv.t

fails hard in cperl

@chocolateboy
Copy link
Collaborator

I wasn't aware of cperl. Very interesting!

Minor nit: I think I'd use:

OP_SPARE(op)

rather than:

op->OP_SPARE

I don't think I've seen the latter style used before.

the called function is also enterxssub, the faster XS variant,
not entersub

What happens if the sub is redefined from an XSUB to a regular sub? Does OP_ENTERXSSUB handle that?

@rurban
Copy link
Author

rurban commented Dec 11, 2016 via email

@rurban
Copy link
Author

rurban commented Dec 11, 2016

Now only 2 commits

@rurban
Copy link
Author

rurban commented Dec 11, 2016

BTW: I didn't see ribasushi's PR #2. His looks better. Just take my 1st XS commit then.

@chocolateboy
Copy link
Collaborator

chocolateboy commented Dec 11, 2016

His looks better.

Thanks. I've merged it.

Reini Urban added 2 commits December 11, 2016 22:35
in cperl. cperl has no spare bit. we can use that one.
the called function is also enterxssub, the faster XS variant,
not entersub.
This todo_skip syntax in fails hard in cperl with a typed Test::More.
@chocolateboy
Copy link
Collaborator

haven’t stepped through it yet.

Did you get a chance to look into this?

Just take my 1st XS commit then.

Has this been rebased against #2? If not, please make a seperate PR.

@rurban
Copy link
Author

rurban commented Feb 22, 2017

My latest fix is in my distroprefs. rurban/distroprefs@0863fd9

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

Successfully merging this pull request may close these issues.

2 participants