Skip to content

<regex>: Successful negative lookahead assertions do not matche to capture groups #5269

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

<regex>: Successful negative lookahead assertions do not matche to capture groups #5269

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b5f972
capture groups in negative lookahead assertions are never matched
alexprabhatbara Feb 4, 2025
2a078d3
reset state after a successful negative assertion
alexprabhatbara Feb 5, 2025
331e2cb
revert penultimate commit change
alexprabhatbara Feb 5, 2025
82e96b6
necessary optimizations
alexprabhatbara Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions stl/inc/regex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,7 @@ public:
template <class _Iter>
char_class_type lookup_classname(_Iter _First, _Iter _Last, bool _Icase = false) const {
// map [_First, _Last) to character class mask value
#define _REGEX_CHAR_CLASS_NAME(n, c) \
{ \
n, L##n, static_cast<unsigned int>(_STD size(n) - 1), c \
}
#define _REGEX_CHAR_CLASS_NAME(n, c) {n, L##n, static_cast<unsigned int>(_STD size(n) - 1), c}
static constexpr _Cl_names _Names[] = {
// map class names to numeric constants
_REGEX_CHAR_CLASS_NAME("alnum", _Ch_alnum),
Expand Down Expand Up @@ -3575,15 +3572,14 @@ bool _Matcher<_BidIt, _Elem, _RxTraits, _It>::_Match_pat(_Node_base* _Nx) { // c
case _N_neg_assert:
case _N_assert:
{ // check assert
_It _Ch = _Tgt_state._Cur;
bool _Neg = _Nx->_Kind == _N_neg_assert;
_Bt_state_t<_It> _St = _Tgt_state;
if (_Match_pat(static_cast<_Node_assert*>(_Nx)->_Child) == _Neg) {
// restore initial state and indicate failure
_Tgt_state = _St;
Copy link
Contributor

Choose a reason for hiding this comment

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

Pre-existing: This state reset after a match failure is both unnecessary and pointless, so I think it should be removed. No caller of _Match_pat() can assume that the match state hasn't changed after failure, because the function does not fully reset the state after trying to match two or more NFA nodes anyway.

_Failed = true;
} else {
_Tgt_state._Cur = _Ch;
} else if (!_Neg) {
_Tgt_state._Cur = _St._Cur;
Comment on lines +3581 to +3582
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not equivalent to the prior commit. Now the state is no longer being reset for a successful negative lookahead assertion at all.

(My comment about a superfluous _Tgt_state = _St; statement points a different line, namely the one in the failure case. The one removed by the last commit in the successful case isn't superfluous at all.)

This suggests we really need more test coverage for negative assertions if no test catches this.

}

break;
Expand Down