search

doy / package-stash-xs

faster and more correct implementation of the Package::Stash API
4 stars 3 forks source link

Don't force unicode when validating package name #4

Open haarg opened 9 years ago

haarg commented 9 years ago

Incoming package stash names usually don't contain unicode, and we already reject them if they do. Since we reject them, doing unicode matching serves no purpose. Don't UTF8 enable the extra sv given to pregexec, and initialize the op_pmdynflags flags to 0 on perl 5.8.

This should serve as a slight optimization to execution time, and will prevent utf8.pm from being automatically loaded. On perl 5.8.6 and below, loading utf8.pm during the regex execution doesn't work correctly, and will cause the regex to fail against some valid inputs.

haarg commented 9 years ago

This fixes #3 and RT#74151

karenetheridge commented 9 years ago

we already reject them if they do

This sounds like a TODO item for a subsequent PR :) Unicode in package names has technically been allowed since perl 5.16. (See Acme-LookOfDisapproval for a module to test against.)

ribasushi commented 8 years ago

@haarg Is this PR still relevant if the validation behavior https://github.com/doy/package-stash/issues/15 is about to be reverted? I.e. is there value in releasing this patch to CPAN as-is in the interim?

haarg commented 8 years ago

Removing the SvUTF8_on is something we'll want even with the validation removed. Shipping this patch as is would at least bring Package::Stash::XS up to the level of "doesn't crash" that Package::Stash::PP has.

ribasushi commented 8 years ago

@genehack With the above remark I have no outstanding concerns on this PR. It is good to go (and potentially even ship).

karenetheridge commented 5 years ago

It would be nice to get this out the door. @doy, how do you feel about passing on a comaint bit or two?

karenetheridge commented 5 years ago

This distribution now lives at https://github.com/moose/Package-Stash-XS -- let's finish this up there.

karenetheridge commented 3 years ago

I have comaint on Package::Stash(::(XS|PP)?. Am happy to ship patches.