Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 37 additions & 19 deletions .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,32 +75,50 @@ jobs:
- name: Compare benchmarks
id: benchstat
run: |
# Full benchstat output to file (for artifact)
benchstat base-bench.txt pr-bench.txt > full-comparison.txt 2>&1 || echo "benchstat comparison failed" > full-comparison.txt

# Extract summary (geomean line) and check for regressions
GEOMEAN=$(grep -E "^geomean" full-comparison.txt | head -1 || echo "")
REGRESSIONS=$(grep -E "\+[0-9]+\.[0-9]+%" full-comparison.txt | grep -v "~" | head -10 || echo "")

# Build concise PR comment
echo "## Benchmark Comparison" > comparison.md
echo "" >> comparison.md
echo "Comparing \`${{ github.event.pull_request.base.ref }}\` (base) vs PR #${{ github.event.pull_request.number }}" >> comparison.md
echo "" >> comparison.md
echo "<details>" >> comparison.md
echo "<summary>Click to expand benchmark results</summary>" >> comparison.md
echo "" >> comparison.md
echo "\`\`\`" >> comparison.md
benchstat base-bench.txt pr-bench.txt >> comparison.md 2>&1 || echo "benchstat comparison failed" >> comparison.md
echo "\`\`\`" >> comparison.md
echo "" >> comparison.md
echo "</details>" >> comparison.md
echo "Comparing \`${{ github.event.pull_request.base.ref }}\` → PR #${{ github.event.pull_request.number }}" >> comparison.md
echo "" >> comparison.md
echo "---" >> comparison.md
echo "" >> comparison.md
echo "**Legend:**" >> comparison.md
echo "- \`~\` = no significant change (within noise)" >> comparison.md
echo "- \`-X%\` = X% faster (improvement)" >> comparison.md
echo "- \`+X%\` = X% slower (regression)" >> comparison.md
echo "" >> comparison.md
echo "> **Note:** CI runners have ~10-20% variance. Only regressions >30% are reliably detected." >> comparison.md

if [ -n "$GEOMEAN" ]; then
echo "**Summary:** \`$GEOMEAN\`" >> comparison.md
echo "" >> comparison.md
fi

if [ -n "$REGRESSIONS" ]; then
echo "⚠️ **Potential regressions detected:**" >> comparison.md
echo "\`\`\`" >> comparison.md
echo "$REGRESSIONS" >> comparison.md
echo "\`\`\`" >> comparison.md
echo "" >> comparison.md
else
echo "✅ No significant regressions detected." >> comparison.md
echo "" >> comparison.md
fi

echo "> Full results available in workflow artifacts. CI runners have ~10-20% variance." >> comparison.md
echo "> For accurate benchmarks, run locally: \`./scripts/bench.sh --compare\`" >> comparison.md

# Store for comment
cat comparison.md

- name: Upload benchmark results
uses: actions/upload-artifact@v4
with:
name: benchmark-comparison
path: |
base-bench.txt
pr-bench.txt
full-comparison.txt
retention-days: 30

- name: Find existing comment
uses: peter-evans/find-comment@v3
id: fc
Expand Down
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

---

## [0.8.11] - 2025-12-08

### Fixed
- **Issue #24: ReverseAnchored patterns return wrong result on first call**
- Pattern `a$` on input "ab" returned `true` on first call, `false` on subsequent calls
- Root cause: `determinize()` returned error for dead states, triggering incorrect NFA fallback
- The fallback PikeVM was created from reverse NFA but received unreversed input
- Fix: `determinize()` now returns `(nil, nil)` for dead states (not an error condition)
- Also fixed: empty string handling and strategy selection for patterns with start anchors
- Files: `dfa/lazy/lazy.go`, `meta/reverse_anchored.go`, `meta/strategy.go`, `nfa/compile.go`

---

## [0.8.10] - 2025-12-07

### Fixed
- **Issue #8: Inline flags `(?s:...)`, `(?i:...)` now work correctly**
- `compileAnyChar()` was checking global config instead of trusting the Op type from parser
- Now correctly produces `OpAnyChar` (matches newlines) vs `OpAnyCharNotNL` based on inline flags
- Examples: `(?s:^a.*c$)` matches `"a\nb\nc"`, `a(?s:.)b` matches `"a\nb"`
- AWK integration: wrap patterns with `(?s:...)` for AWK-like behavior where `.` matches newlines

---

## [0.8.9] - 2025-12-07

### Fixed
Expand Down
6 changes: 4 additions & 2 deletions dfa/lazy/lazy.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,8 +601,10 @@ func (d *DFA) determinize(current *State, b byte) (*State, error) {
if len(nextNFAStates) == 0 {
// Cache the dead state transition to avoid re-computation
current.AddTransition(b, DeadState)
// Return nil state with a specific error to indicate dead state
return nil, &DFAError{Kind: NFAFallback, Message: "dead state (no transitions)"}
// Return nil state with NO error - dead state is NOT an error condition.
// This follows the documented behavior: (nil, nil) for dead state.
// Returning an error here would incorrectly trigger NFA fallback.
return nil, nil //nolint:nilnil // dead state is valid, not an error
}

// Check if we've exceeded determinization limit
Expand Down
31 changes: 23 additions & 8 deletions meta/reverse_anchored.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ import (
// // Forward: 340 seconds (tries match at every position)
// // Reverse: ~1 millisecond (one match attempt from end)
type ReverseAnchoredSearcher struct {
reverseNFA *nfa.NFA
reverseDFA *lazy.DFA
pikevm *nfa.PikeVM
reverseNFA *nfa.NFA
reverseDFA *lazy.DFA
pikevm *nfa.PikeVM
forwardPikevm *nfa.PikeVM // For empty string matching (reverse NFA has issues with empty)
}

// NewReverseAnchoredSearcher creates a reverse searcher from forward NFA.
Expand All @@ -55,10 +56,15 @@ func NewReverseAnchoredSearcher(forwardNFA *nfa.NFA, config lazy.Config) (*Rever
// Create PikeVM for fallback (when DFA cache is full)
pikevm := nfa.NewPikeVM(reverseNFA)

// Create forward PikeVM for empty string matching
// Reverse NFA has issues with empty strings and certain alternations
forwardPikevm := nfa.NewPikeVM(forwardNFA)

return &ReverseAnchoredSearcher{
reverseNFA: reverseNFA,
reverseDFA: reverseDFA,
pikevm: pikevm,
reverseNFA: reverseNFA,
reverseDFA: reverseDFA,
pikevm: pikevm,
forwardPikevm: forwardPikevm,
}, nil
}

Expand All @@ -81,8 +87,14 @@ func NewReverseAnchoredSearcher(forwardNFA *nfa.NFA, config lazy.Config) (*Rever
// Match in reverse: [0:3] = "cba"
// Convert to forward: [3:6] = "abc"
func (s *ReverseAnchoredSearcher) Find(haystack []byte) *Match {
// For empty strings, use forward PikeVM
// Reverse NFA has issues with empty strings and certain alternations
if len(haystack) == 0 {
return nil
start, end, matched := s.forwardPikevm.Search(haystack)
if !matched {
return nil
}
return NewMatch(start, end, haystack)
}

// Quick check: use zero-allocation reverse DFA scan
Expand Down Expand Up @@ -132,8 +144,11 @@ func reverseBytes(b []byte) []byte {
// - No Match object allocation
// - Early termination
func (s *ReverseAnchoredSearcher) IsMatch(haystack []byte) bool {
// For empty strings, use forward PikeVM
// Reverse NFA has issues with empty strings and certain alternations
if len(haystack) == 0 {
return false
_, _, matched := s.forwardPikevm.Search(haystack)
return matched
}

// Use reverse DFA to scan backward from end to start
Expand Down
15 changes: 11 additions & 4 deletions meta/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,21 @@ func SelectStrategy(n *nfa.NFA, re *syntax.Regexp, literals *literal.Seq, config
// 3. Have DFA enabled
// This converts O(n*m) forward search to O(m) reverse search
//
// Note: Start-anchored patterns (^...) are now handled correctly by the DFA.
// The DFA's epsilonClosure properly handles StateLook assertions by checking
// which look assertions are satisfied at each position (see dfa/lazy/look.go).
// Note: We must avoid UseReverseAnchored for patterns that contain any start
// anchor (^ or \A), even in alternations like `^a?$|^b?$`. The reverse DFA
// cannot properly handle start anchors and would produce false positives.
isStartAnchored := n.IsAlwaysAnchored()
isEndAnchored := re != nil && nfa.IsPatternEndAnchored(re)
hasStartAnchor := re != nil && nfa.IsPatternStartAnchored(re)

if re != nil && config.EnableDFA {
if isEndAnchored && !isStartAnchored {
// Only use reverse search if:
// 1. Pattern is end-anchored ($)
// 2. Pattern is NOT fully start-anchored (not always starting at position 0)
// 3. Pattern does NOT contain any start anchor (^ or \A) - this catches
// alternations like `^a?$|^b?$` where IsAlwaysAnchored() returns false
// but the pattern still has start anchors that need proper handling
if isEndAnchored && !isStartAnchored && !hasStartAnchor {
// Perfect candidate for reverse search
// Example: "pattern.*suffix$" on large haystack
// Forward: O(n*m) tries, Reverse: O(m) one try
Expand Down
38 changes: 38 additions & 0 deletions nfa/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -1174,3 +1174,41 @@ func containsEndAnchor(re *syntax.Regexp) bool {
}
return false
}

// IsPatternStartAnchored checks if ANY branch of the pattern starts with ^ or \A.
// This is used to prevent UseReverseAnchored strategy selection for patterns like
// `^a?$|^b?$` where the start anchor constrains matching positions.
//
// Unlike IsPatternEndAnchored which requires ALL branches to be end-anchored,
// this function returns true if ANY branch has a start anchor, because reverse
// search cannot properly handle partial start anchoring in alternations.
func IsPatternStartAnchored(re *syntax.Regexp) bool {
return containsStartAnchor(re)
}

// containsStartAnchor checks if the AST contains any start anchor (^ or \A)
func containsStartAnchor(re *syntax.Regexp) bool {
switch re.Op {
case syntax.OpBeginText, syntax.OpBeginLine:
return true
case syntax.OpConcat:
// Check all parts of concatenation (start anchor could be in first position)
for _, sub := range re.Sub {
if containsStartAnchor(sub) {
return true
}
}
case syntax.OpAlternate:
// Check all alternatives - if ANY has start anchor, we need to be careful
for _, sub := range re.Sub {
if containsStartAnchor(sub) {
return true
}
}
case syntax.OpCapture, syntax.OpStar, syntax.OpPlus, syntax.OpQuest, syntax.OpRepeat:
if len(re.Sub) > 0 {
return containsStartAnchor(re.Sub[0])
}
}
return false
}
53 changes: 53 additions & 0 deletions regex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,59 @@ func TestEdgeCases(t *testing.T) {
}
}

// TestEndAnchor tests patterns with $ anchor
// Regression test for issue #24: first-call bug with ReverseAnchored patterns
func TestEndAnchor(t *testing.T) {
tests := []struct {
name string
pattern string
input string
want bool
}{
// Basic end anchor
{"a$ matches ending with a", "a$", "ba", true},
{"a$ not matches ending with b", "a$", "ab", false},
{"a$ matches single a", "a$", "a", true},
{"a$ not matches single b", "a$", "b", false},

// Empty string handling
{"^$ matches empty", "^$", "", true},
{"^$ not matches non-empty", "^$", "a", false},
{"$ matches at end of abc", "$", "abc", true},

// Multiple calls should give consistent results (regression for #24)
{"a$ on ab consistent 1", "a$", "ab", false},
{"a$ on ab consistent 2", "a$", "ab", false},
{"a$ on ba consistent 1", "a$", "ba", true},
{"a$ on ba consistent 2", "a$", "ba", true},

// Start anchor combinations
{"^a$ full match a", "^a$", "a", true},
{"^a$ not matches ab", "^a$", "ab", false},
{"^a$ not matches ba", "^a$", "ba", false},

// Alternation with anchors
{"^a?$|^b?$ matches empty", "^a?$|^b?$", "", true},
{"^a?$|^b?$ matches a", "^a?$|^b?$", "a", true},
{"^a?$|^b?$ matches b", "^a?$|^b?$", "b", true},
{"^a?$|^b?$ not matches ab", "^a?$|^b?$", "ab", false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
re := MustCompile(tt.pattern)

// Call multiple times to catch first-call bugs
for i := 0; i < 3; i++ {
got := re.MatchString(tt.input)
if got != tt.want {
t.Errorf("MatchString() call %d = %v, want %v", i+1, got, tt.want)
}
}
})
}
}

// BenchmarkCompile benchmarks compilation
func BenchmarkCompile(b *testing.B) {
patterns := []string{
Expand Down