You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR deals with three related problems for character ranges in case-insensitive mode:
_Builder::_Add_range() casts the character bounds to unsigned int. As a consequence, characters with negative numeric values are not added to the bitmap, but rather to the _Large list of characters. This means that these characters are not found during matching. (Note the suspiciously different casts in the else branch for the case-sensitive case.)
The parser fails to reject some empty ranges in case-insensitive mode such as [Z-a] (= [z-a]).
When both the collate and icase flags are set, there is an unnecessary call to translate the bounds by _Traits.translate() first before passing them to _Traits.translate_nocase(). The standard says in [re.grammar]/14.1 and 14.2 that it is sufficient to call translate_nocase() only. (See also _Builder::_Add_char(), which already follows the Standard in this regard.)
The PR moves the entire character translation into the parser so that empty ranges can be reliably diagnosed there in case-insensitive mode as well. It also fixes the unsigned cast and removes the unnecessary translate() call.
The test deliberately does not use any manual signed/unsigned casts, but leaves all of these casts to char_traits to avoid getting the casts similarly wrong in <regex> and the test.
The reason will be displayed to describe this comment to others. Learn more.
Note that there is still a (pre-existing) bug in this error check when the collate flag is set: The bounds should be transformed by _Traits.transform() before performing the comparison.
However, the matcher fails to do this transform() dance as well, so I think this should rather be fixed for both matcher and parser in a coordinated fashion in a follow-up PR. (But I will adjust the PR if you rather prefer to fix this check completely now.)
bugSomething isn't workingregexmeow is a substring of homeowner
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR deals with three related problems for character ranges in case-insensitive mode:
_Builder::_Add_range()
casts the character bounds tounsigned int
. As a consequence, characters with negative numeric values are not added to the bitmap, but rather to the_Large
list of characters. This means that these characters are not found during matching. (Note the suspiciously different casts in the else branch for the case-sensitive case.)[Z-a]
(=[z-a]
).collate
andicase
flags are set, there is an unnecessary call to translate the bounds by_Traits.translate()
first before passing them to_Traits.translate_nocase()
. The standard says in [re.grammar]/14.1 and 14.2 that it is sufficient to calltranslate_nocase()
only. (See also_Builder::_Add_char()
, which already follows the Standard in this regard.)The PR moves the entire character translation into the parser so that empty ranges can be reliably diagnosed there in case-insensitive mode as well. It also fixes the unsigned cast and removes the unnecessary
translate()
call.The test deliberately does not use any manual signed/unsigned casts, but leaves all of these casts to
char_traits
to avoid getting the casts similarly wrong in<regex>
and the test.