-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Drop MATCH_INVALID_UTF
#13313
Drop MATCH_INVALID_UTF
#13313
Conversation
# | ||
# Unsupported with PCRE. | ||
# | ||
# NOTE: This option was introduced in PCRE2 10.34 but a bug that can lead to an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That deserves a stronger marker IMO.
# NOTE: This option was introduced in PCRE2 10.34 but a bug that can lead to an | |
# WARNING: This option was introduced in PCRE2 10.34 but a bug that can lead to an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine. If you want to use a feature of any library you must make sure it's supported in the library version you're linking against. That's a general axiom. Here we're just pointing out the relevant versions for this particular option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're pointing not only the version but also the fact it contains a rather serious bug.
This patch drops the implicit
MATCH_INVALID_UTF
option which was added as part of a bug fix in #13240.Instead this option is now available in
Regex::CompileOptions
for opt-in.Similarly,
NO_CHECK_UTF
members are added toRegex::CompileOptions
andRegex::MatchOptions
.Regex::CompileOptions::NO_CHECK_UTF
is identical to the already existing but undocumentedNO_CHECK_UTF8
member.Resolves #13312