Merge pull request #117 from coregx/feature/issue116-reverse-suffix-fix

fix: reject alternation patterns from ReverseSuffixSet (Issue #116)

Author: kolkov

CHANGELOG.md

@@ -12,6 +12,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ARM NEON SIMD support (waiting for Go 1.26 native SIMD)
 - SIMD prefilter for CompositeSequenceDFA (#83)
 
+## [0.12.2] - 2026-02-16
+
+### Fixed
+- **Alternation patterns misrouted to ReverseSuffixSet** (Issue #116) —
+  Patterns like `[cgt]gggtaaa|tttaccc[acg]` (alternation without `.*` prefix)
+  were incorrectly routed to UseReverseSuffixSet strategy, which assumed
+  match always starts at position 0. This produced wrong match boundaries
+  (e.g., `[0, 37976]` instead of `[3, 11]`). Fix: added `isSafeForReverseSuffix`
+  guard before UseReverseSuffixSet selection.
+- **`matchStartZero` optimization too aggressive** — The reverse DFA skip
+  optimization (`matchStartZero`) was enabled for all unanchored patterns,
+  but is only correct for `.*` prefix patterns. Patterns like `[^\s]+\.txt`
+  or `.+\.txt` could return wrong match start positions. Fix: restricted
+  `matchStartZero` to patterns with explicit `OpStar(AnyChar)` prefix.
+
 ## [0.12.1] - 2026-02-15
 
 ### Performance

meta/compile.go

@@ -217,7 +217,7 @@ func buildReverseSearchers(
 
 	case UseReverseSuffix:
 		suffixLiterals := extractor.ExtractSuffixes(re)
-		searcher, err := NewReverseSuffixSearcher(nfaEngine, suffixLiterals, dfaConfig)
+		searcher, err := NewReverseSuffixSearcher(nfaEngine, suffixLiterals, dfaConfig, hasDotStarPrefix(re))
 		if err != nil {
 			result.finalStrategy = UseDFA
 		} else {
@@ -226,7 +226,7 @@ func buildReverseSearchers(
 
 	case UseReverseSuffixSet:
 		suffixLiterals := extractor.ExtractSuffixes(re)
-		searcher, err := NewReverseSuffixSetSearcher(nfaEngine, suffixLiterals, dfaConfig)
+		searcher, err := NewReverseSuffixSetSearcher(nfaEngine, suffixLiterals, dfaConfig, hasDotStarPrefix(re))
 		if err != nil {
 			result.finalStrategy = UseBoth
 		} else {

meta/reverse_suffix.go

@@ -72,6 +72,7 @@ func NewReverseSuffixSearcher(
 	forwardNFA *nfa.NFA,
 	suffixLiterals *literal.Seq,
 	config lazy.Config,
+	matchStartZero bool,
 ) (*ReverseSuffixSearcher, error) {
 	// Get suffix bytes from longest common suffix
 	var suffixBytes []byte
@@ -111,11 +112,9 @@ func NewReverseSuffixSearcher(
 	// Create PikeVM for fallback
 	pikevm := nfa.NewPikeVM(forwardNFA)
 
-	// Check if pattern is unanchored (starts matching from position 0)
-	// For unanchored patterns with .* prefix, match always starts at 0
-	// This allows us to skip the reverse DFA scan entirely!
-	matchStartZero := !forwardNFA.IsAlwaysAnchored()
-
+	// matchStartZero is true only when pattern has .* prefix (e.g., `.*\.txt`).
+	// Only OpStar(AnyChar) guarantees match starts at 0/at — skip reverse DFA.
+	// Other wildcards like .+, [^\s]+, \w{2,8} do NOT guarantee this.
 	return &ReverseSuffixSearcher{
 		forwardNFA:     forwardNFA,
 		reverseNFA:     reverseNFA,

meta/reverse_suffix_set.go

@@ -61,6 +61,7 @@ func NewReverseSuffixSetSearcher(
 	forwardNFA *nfa.NFA,
 	suffixLiterals *literal.Seq,
 	config lazy.Config,
+	matchStartZero bool,
 ) (*ReverseSuffixSetSearcher, error) {
 	if suffixLiterals == nil || suffixLiterals.IsEmpty() {
 		return nil, ErrNoSuffixSet
@@ -103,9 +104,8 @@ func NewReverseSuffixSetSearcher(
 	// Create PikeVM for fallback
 	pikevm := nfa.NewPikeVM(forwardNFA)
 
-	// Check if pattern is unanchored (starts matching from position 0)
-	matchStartZero := !forwardNFA.IsAlwaysAnchored()
-
+	// matchStartZero is true only when pattern has .* prefix (e.g., `.*\.(txt|log|md)`).
+	// Only OpStar(AnyChar) guarantees match starts at 0/at — skip reverse DFA.
 	return &ReverseSuffixSetSearcher{
 		forwardNFA:     forwardNFA,
 		reverseNFA:     reverseNFA,

meta/reverse_suffix_test.go

@@ -391,3 +391,88 @@ func TestReverseSuffix_EdgeCases(t *testing.T) {
 		})
 	}
 }
+
+// TestIssue116_AlternationWithoutWildcard tests that alternation patterns without
+// wildcard prefix (like `[cgt]gggtaaa|tttaccc[acg]`) are NOT routed to
+// UseReverseSuffixSet, which would produce wrong match positions.
+// See: https://github.com/coregx/coregex/issues/116
+func TestIssue116_AlternationWithoutWildcard(t *testing.T) {
+	tests := []struct {
+		name    string
+		pattern string
+		input   string
+		want    []string
+	}{
+		{
+			name:    "original bug report pattern",
+			pattern: `[cgt]gggtaaa|tttaccc[acg]`,
+			input:   "xxxcgggtaaaxxx",
+			want:    []string{"cgggtaaa"},
+		},
+		{
+			name:    "original bug report pattern - second alt",
+			pattern: `[cgt]gggtaaa|tttaccc[acg]`,
+			input:   "xxxtttacccaxxx",
+			want:    []string{"tttaccca"},
+		},
+		{
+			name:    "multiple matches",
+			pattern: `[cgt]gggtaaa|tttaccc[acg]`,
+			input:   "cgggtaaa tttaccca ggggtaaa tttacccg",
+			want:    []string{"cgggtaaa", "tttaccca", "ggggtaaa", "tttacccg"},
+		},
+		{
+			name:    "no match",
+			pattern: `[cgt]gggtaaa|tttaccc[acg]`,
+			input:   "agggtaaa tttacccd",
+			want:    nil,
+		},
+		{
+			name:    "simple alternation without char class",
+			pattern: `foo|bar`,
+			input:   "xxxfooxxxbarxxx",
+			want:    []string{"foo", "bar"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e, err := Compile(tt.pattern)
+			if err != nil {
+				t.Fatalf("Compile(%q) failed: %v", tt.pattern, err)
+			}
+
+			// Verify strategy is NOT UseReverseSuffixSet for these patterns
+			strategy := e.Strategy()
+			if strategy == UseReverseSuffixSet {
+				t.Errorf("pattern %q should NOT use UseReverseSuffixSet strategy", tt.pattern)
+			}
+
+			var got []string
+			haystack := []byte(tt.input)
+			at := 0
+			for at < len(haystack) {
+				start, end, found := e.FindIndicesAt(haystack, at)
+				if !found {
+					break
+				}
+				got = append(got, string(haystack[start:end]))
+				if end > at {
+					at = end
+				} else {
+					at++
+				}
+			}
+
+			if len(got) != len(tt.want) {
+				t.Fatalf("FindAll(%q) = %v (%d matches), want %v (%d matches)",
+					tt.input, got, len(got), tt.want, len(tt.want))
+			}
+			for i := range got {
+				if got[i] != tt.want[i] {
+					t.Errorf("match[%d] = %q, want %q", i, got[i], tt.want[i])
+				}
+			}
+		})
+	}
+}

meta/strategy.go

@@ -574,6 +574,30 @@ func isDigitRunSkipSafe(re *syntax.Regexp) bool {
 //   - Internal anchors: `0?^0`, `a$b` - position constraints don't reverse
 //   - Star of CharClass: `[^\s]*suffix` - zero-width match edge cases
 
+// hasDotStarPrefix checks if a pattern has `.*` as its first subexpression.
+// Only `.*` (OpStar of AnyChar/AnyCharNotNL) guarantees matching from position 0
+// to any position without fail. Used to determine if reverse DFA scan can be skipped.
+//
+// Returns true for: `.*\.txt`, `(.*)\.txt`
+// Returns false for: `.+\.txt`, `[^\s]+\.txt`, `\w{2,8}\.txt`, `foo|bar`
+func hasDotStarPrefix(re *syntax.Regexp) bool {
+	// Unwrap captures
+	for re.Op == syntax.OpCapture && len(re.Sub) > 0 {
+		re = re.Sub[0]
+	}
+	if re.Op != syntax.OpConcat || len(re.Sub) < 2 {
+		return false
+	}
+	first := re.Sub[0]
+	// Unwrap captures on first sub
+	for first.Op == syntax.OpCapture && len(first.Sub) > 0 {
+		first = first.Sub[0]
+	}
+	// .* = OpStar(OpAnyChar or OpAnyCharNotNL)
+	return first.Op == syntax.OpStar && len(first.Sub) > 0 &&
+		(first.Sub[0].Op == syntax.OpAnyChar || first.Sub[0].Op == syntax.OpAnyCharNotNL)
+}
+
 // isWildcardSubexpression checks if a subexpression acts as a "wildcard" that can
 // consume variable-length input. Used by isSafeForReverseSuffix to identify patterns
 // suitable for reverse suffix search.
@@ -995,7 +1019,9 @@ func selectReverseStrategy(n *nfa.NFA, re *syntax.Regexp, literals *literal.Seq,
 
 	// No common suffix (LCS empty), but check if multiple suffix literals available
 	// for Teddy multi-suffix prefilter. This handles patterns like `.*\.(txt|log|md)`.
-	if shouldUseReverseSuffixSet(literals, suffixLiterals) {
+	// Must also pass isSafeForReverseSuffix to reject patterns without wildcard prefix
+	// (e.g., `[cgt]gggtaaa|tttaccc[acg]` — Issue #116).
+	if isSafeForReverseSuffix(re) && shouldUseReverseSuffixSet(literals, suffixLiterals) {
 		return UseReverseSuffixSet
 	}