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

[alexprabhat99]

Fixes: #5245

[AlexGuteniev]

I'd recommend running tests locally.
You can use regex to run all regex tests, this command works for me:
python tests\utils\stl-lit\stl-lit.py -Dnotags=ASAN ..\..\llvm-project\libcxx\test ..\..\tests\std ..\..\tests\tr1 --filter=(regex^|/re\.)

[muellerj2]

This deletion does not just correct the matches for negative assertions. With this change, no capture group at all will ever be matched.

Changes to fix #5245 actually have to be applied here:

STL/stl/inc/regex

Lines 3575 to 3590 in fc15609

	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;
	_Failed = true;
	} else {
	_Tgt_state._Cur = _Ch;
	}
	break;
	}

Specifically, _Tgt_state._Cur = _Ch; insufficiently resets the state after a successful negative assertion. (But it is the appropriate reset for positive assertions.)

[alexprabhat99]

I have restored the initial state after a successful negative assertion.

[muellerj2]

I think a test (like the one in #5245) should be added to verify that the behavior is correct after this PR.

Edit: It would also be nice to add test coverage which verifies that captures are recorded correctly for positive assertions as well.

[muellerj2]

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.

[muellerj2]

The opening brace after else leads to unnecessary nesting. You can just remove the braces and write else if instead.

[muellerj2]

Pre-existing: We can assign _St._Cur to _Tgt_state._Cur here instead of _Ch and eliminate the local variable _Ch. This should also decrease the amount of stack this function uses per call a bit, slightly reducing the danger of stack overflow (as in #997).

[muellerj2]

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.