Allow captures during lookahead #723
Conversation
This fixes an issue where capture groups inside a positive lookahead
were being reset even upon successful matching of the lookahead. For
example, with the pattern `/(?=(\d))/`, matching against a string like
`"abc1"` should result in the output `("", "1")`. However, accessing
the output traps instead, since the range data for capture 1 is
missing even on success.
This change resolves the issue by adding a boolean payload to the
`fail` instruction that indicates whether to preserve captures when
resetting the matching state, which allows any captures inside a
lookahead to persist after success.
Fixes #713.
| init(bool: BoolRegister) { | ||
| self.init(bool) | ||
| init(bool: Bool) { | ||
| self.init(bool ? 1 : 0, 0) | ||
| } | ||
| var bool: BoolRegister { | ||
| interpret() | ||
| var boolPayload: Bool { | ||
| interpret(as: TypedInt<Bool>.self) == 1 |
There was a problem hiding this comment.
Okay to switch this to a payload access? It doesn't look like we actually have a Boolean register.
There was a problem hiding this comment.
Sure, if there's no need to have a value that's stored and updated
| mutating func buildFail(preservingCaptures: Bool = false) { | ||
| instructions.append(.init(.fail, .init(bool: preservingCaptures))) |
There was a problem hiding this comment.
What would you think about renaming fail to backtrack or something similar? I always have to remember that the fail instruction isn't an overall failure, just a failure of the immediate execution track.
There was a problem hiding this comment.
Sure, if we can do it pervasively and consistently
milseman
left a comment
milseman
left a comment
There was a problem hiding this comment.
Overall LGTM, with some comments
| init(bool: BoolRegister) { | ||
| self.init(bool) | ||
| init(bool: Bool) { | ||
| self.init(bool ? 1 : 0, 0) | ||
| } | ||
| var bool: BoolRegister { | ||
| interpret() | ||
| var boolPayload: Bool { | ||
| interpret(as: TypedInt<Bool>.self) == 1 |
There was a problem hiding this comment.
Sure, if there's no need to have a value that's stored and updated
| mutating func buildFail(preservingCaptures: Bool = false) { | ||
| instructions.append(.init(.fail, .init(bool: preservingCaptures))) |
There was a problem hiding this comment.
Sure, if we can do it pervasively and consistently
|
@swift-ci Please test |
This fixes an issue where capture groups inside a positive lookahead were being reset even upon successful matching of the lookahead. For example, with the pattern
/(?=(\d))/, matching against a string like"abc1"should result in the output("", "1"). However, accessing the output traps instead, since the range data for capture 1 is missing even on success.This change resolves the issue by adding a boolean payload to the
failinstruction that indicates whether to preserve captures when resetting the matching state, which allows any captures inside a lookahead to persist after success.Fixes #713.