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

Implement Regex engine on PCRE2 #12856

Merged
merged 7 commits into from
Dec 22, 2022
Merged

Implement Regex engine on PCRE2 #12856

merged 7 commits into from
Dec 22, 2022

Conversation

straight-shoota
Copy link
Member

This is a re-issue of #12840 which was reverted due to errors. The updated branch includes several bugfixes and enhancements.

This is an initial implementation of a Regex engine backend based on PCRE2 (see #12790).

All existing regex specs succeed with PCRE2 🎉

This is an MVP and does not include JIT compilation (I'm planning a follow-up).

PCRE2 is currently not automatically selected. It's opt-in via the compiler flag -Dforce_pcre2.

If you have Crystal software that makes heavy use of regular expressions, please give this patch a try in order to identify potential issues in practical use cases. Performance comparisons are also welcome.

  • We're using the 8-byte version of PCRE2, so the library name is actually libpcre2-8 (this is also the package name on some systems).
  • pkg-config needs the .pc configuration which may require installing the devel package of libpcre2.

Significant changes from #12840:

  • Added CI jobs to test against both PCRE and PCRE2
  • Removed PCRE2 as default. It's only available as opt-in for now.
  • Allocate matchdata using the GC so it gets automatically collected
  • Improved error message for match errors

@jwoertink
Copy link
Contributor

Nice! So this one seems to be working better than the last one. I ran the specs on https://github.com/jwoertink/CrystalEmail to test

❯ /home/jeremy/Development/crystal/lang/bin/crystal spec 
Using compiled compiler at /home/jeremy/Development/crystal/lang/.build/crystal
....

Finished in 222 microseconds
4 examples, 0 failures, 0 errors, 0 pending


❯ /home/jeremy/Development/crystal/lang/bin/crystal spec -Duse_pcre2
Using compiled compiler at /home/jeremy/Development/crystal/lang/.build/crystal
....

Finished in 136 microseconds
4 examples, 0 failures, 0 errors, 0 pending

@HertzDevil
Copy link
Contributor

No issues on my M2 via nix-shell (requires adding pcre2 to stdLibDeps but that's about it)

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor question

end
end

class_getter general_context do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be private (or protected)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire Regex::PCRE2 namespace is :nodoc: so I think visibility doesn't matter much. I suppose it could be protected, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's just nitpicking, I was wondering if there were a reason to leave it public

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.

4 participants