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

Patches from split #49

Closed
wants to merge 26 commits into from
Closed

Conversation

haarg
Copy link
Member

@haarg haarg commented Sep 13, 2016

These are various patches I wrote or took from other sources when attempting to split the modules into separate dists.

@dur-randir
Copy link
Contributor

Sub::Util part carries on all the problems accumulated around it:

  • these don't compile before 5.16
  • these fail some tests before 5.16
  • these do not fix \0 mishandling
  • these are broken under debugger

@haarg
Copy link
Member Author

haarg commented Sep 14, 2016

Updating ppport.h fixes the issues with <5.16. This patchset shouldn't cause any additional issues with \0 in sub names, so that can be addressed separately.

@dur-randir Can you elaborate on the issues with the debugger?

@dur-randir
Copy link
Contributor

dur-randir commented Sep 14, 2016

This patchset shouldn't cause any additional issues with \0 in sub names

If the intention is not a full binary/utf8 support here, then the commit "Support binary data and unicode in symbol names" has a misleading name.

Can you elaborate on the issues with the debugger?

%DB::sub entries are not correctly set for the same cases.

@haarg
Copy link
Member Author

haarg commented Sep 14, 2016

What "same cases"?

@dur-randir
Copy link
Contributor

What "same cases"?

Setting binary/utf8 name for a function.

@haarg
Copy link
Member Author

haarg commented Sep 14, 2016

Re-reviewing the patches, they do handle \0 in sub names. %DB::sub does have some edge cases though.

haarg and others added 24 commits January 27, 2017 10:53
On perl 5.8.0, storing PL_sv_undef directly in a hash will result in
hv_exists returning false for that key.  Use PL_sv_yes instead to work
around this.
This is scaffolding to sanity-check our assumptions about perl: that
anything we *can* eval is named by the interpreter as expected. This
test passes on 5.6.1+ on both 32 and 64 bit perls.
Reveals incorrect handling of
- ' (\x27) on all perls
- \0 on perl > 5.14
- Unicode on perl > 5.16
When a stash is deleted, the subs attached to it will lose track of
various bits of information.  What gets lots depends on the perl
version.  Rather than using a very loose check or enumerating versions,
use the value returned by caller, which we should always be matching.
subname is able to give better names than caller in some versions, and
there isn't any other source for the accurate names than Devel::Peek.
The only reasonable option is to check based on the version numbers.
Rather than just checking for existence, compare with what was actually
created.  Also skip test when %DB::sub isn't populated (such as perl
5.8.0 when run without -d).
@haarg
Copy link
Member Author

haarg commented Jan 28, 2017

I've updated this to fix the %DB::sub issues and included some other patches from Sub::Name.

@ribasushi
Copy link

@haarg out of curiosity does the above address @chipdude's forgotten issue: https://rt.cpan.org/Ticket/Display.html?id=65540#txn-893983

@haarg
Copy link
Member Author

haarg commented Jan 3, 2018

Split into #63, #64, #65, #66, #67

@haarg haarg closed this Jan 3, 2018
@haarg haarg deleted the patches-from-split branch January 5, 2018 13:51
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

Successfully merging this pull request may close these issues.

6 participants