[go] regexp: fix pathological slowdown in String for negated classes

5 views
Skip to first unread message

Nicholas Feinberg (Gerrit)

unread,
Jul 10, 2026, 7:17:56 PM (7 days ago) Jul 10
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Nicholas Feinberg has uploaded the change for review

Commit message

regexp: fix pathological slowdown in String for negated classes

The existing implementation of regexp.String() is very inefficient
for large classes, such as those generated by negating small classes.
(For example, [^.] or \W.) To determine whether it can apply the
case-insensitive ?i flag to such classes, calcFlags() iterates
across every case-fold variant of every rune within such classes.

Avoid this by checking if a class covers the majority of the
foldable space (between minFold and maxFold, 'A' and 0x1e943) and,
if so, check runes *not* within the class to see if they fold to
a rune which *is* in the class.

Fixes #69303
Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334

Change diff

diff --git a/src/regexp/syntax/regexp.go b/src/regexp/syntax/regexp.go
index 4994928..f1763eb 100644
--- a/src/regexp/syntax/regexp.go
+++ b/src/regexp/syntax/regexp.go
@@ -147,18 +147,10 @@
 		return 0, 0
 
 	case OpCharClass:
-		// If literal is fold-sensitive, return 0, flagI - (?i) has been compiled out.
-		// If literal is not fold-sensitive, return 0, 0.
-		for i := 0; i < len(re.Rune); i += 2 {
-			lo := max(minFold, re.Rune[i])
-			hi := min(maxFold, re.Rune[i+1])
-			for r := lo; r <= hi; r++ {
-				for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
-					if !(lo <= f && f <= hi) && !inCharClass(f, re.Rune) {
-						return 0, flagI
-					}
-				}
-			}
+		// If literal is fold-sensitive, forbid flagI - it's not safe to
+		// include this with other subexpressions that use (?i).
+		if foldSensitive(re) {
+			return 0, flagI
 		}
 		return 0, 0
 
@@ -221,6 +213,77 @@
 	}
 }
 
+func runesInFoldableRange(re *Regexp) int32 {
+	var total int32
+	for i := range len(re.Rune) / 2 {
+		lowRune := re.Rune[i*2]
+		highRune := re.Rune[i*2+1]
+		if lowRune <= maxFold && highRune >= minFold {
+			total += min(highRune, maxFold) - max(lowRune, minFold) + 1
+		}
+	}
+	return total
+}
+
+func includesMostFoldableRunes(re *Regexp) bool {
+	if len(re.Rune) == 0 {
+		return false
+	}
+	// To avoid having to iterate in the common case, check the distance from
+	// the highest to lowest rune (which will generally be very small) before
+	// checking each range.
+	totalRangeWidth := min(maxFold, re.Rune[len(re.Rune)-1]) - max(minFold, re.Rune[0])
+	const foldableRangeSize = maxFold - minFold
+	return totalRangeWidth >= foldableRangeSize/2 &&
+		runesInFoldableRange(re) >= foldableRangeSize/2
+}
+
+func foldSensitive(re *Regexp) bool {
+	// Being fold-sensitive means that some rune within one of the regexp's
+	// ranges has a fold variant which is not in any of its ranges.
+
+	// Many common expressions, such as \W, include a huge range of runes,
+	// and exclude only a few runes within [minFold, maxFold]. In these
+	// cases, to avoid needing to check hundreds of thousands of runes,
+	// loop over runes which are *not* in the regexp, and check if *they*
+	// have a fold variant which *is* in the regexp (a symmetric property).
+	if !includesMostFoldableRunes(re) {
+		for i := range len(re.Rune) / 2 {
+			low := max(minFold, re.Rune[i*2])
+			high := min(maxFold, re.Rune[i*2+1])
+			for r := low; r <= high; r++ {
+				for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
+					if !(low <= f && f <= high) && !inCharClass(f, re.Rune) {
+						return true
+					}
+				}
+			}
+		}
+		return false
+	}
+
+	for i := range len(re.Rune)/2 + 1 {
+		// Check outside the class.
+		// [minFold, re.Rune[0]-1], [re.Rune[1]+1, re.Rune[2]-1], ..., [re.Rune[len(re.Rune)-1]+1, maxFold]
+		low := int32(minFold)
+		high := int32(maxFold)
+		if i > 0 {
+			low = max(low, re.Rune[i*2-1]+1)
+		}
+		if i*2 < len(re.Rune) {
+			high = min(high, re.Rune[i*2]-1)
+		}
+		for r := low; r <= high; r++ {
+			for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
+				if inCharClass(f, re.Rune) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
 // writeRegexp writes the Perl syntax for the regular expression re to b.
 func writeRegexp(b *strings.Builder, re *Regexp, f printFlags, flags map[*Regexp]printFlags) {
 	f |= flags[re]
diff --git a/src/regexp/syntax/regexp_test.go b/src/regexp/syntax/regexp_test.go
new file mode 100644
index 0000000..9c7c705
--- /dev/null
+++ b/src/regexp/syntax/regexp_test.go
@@ -0,0 +1,78 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syntax
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestRunesInFoldableRange(t *testing.T) {
+	for _, tt := range []struct {
+		regexp string
+		want   int32
+	}{
+		{`[a-z]`, 26},
+		{`[a-bd-eg-h]`, 6},
+		{`[\x00-A]`, 1},              // end at minFold
+		{`[\x{1e943}-\x{1e949}]`, 1}, // start at maxFold
+	} {
+		re, err := Parse(tt.regexp, MatchNL|Perl&^OneLine)
+		if err != nil {
+			t.Errorf("Parse(%#q) = error %v", tt.regexp, err)
+			continue
+		}
+		if got := runesInFoldableRange(re); got != tt.want {
+			t.Errorf("runesInFoldableRange(%#q) = %d, want %d", tt.regexp, got, tt.want)
+		}
+	}
+}
+
+func TestIncludesMostFoldableRunes(t *testing.T) {
+	for _, tt := range []struct {
+		regexp string
+		want   bool
+	}{
+		{``, false},
+		{`[A-z]`, false},
+		{`[A-\x{10000}]`, true},
+		{`\W`, true},
+		{`[\x{1e900}-\x{3e949}]`, false}, // large, but mostly non-foldable
+	} {
+		re, err := Parse(tt.regexp, MatchNL|Perl&^OneLine)
+		if err != nil {
+			t.Errorf("Parse(%#q) = error %v", tt.regexp, err)
+			continue
+		}
+		if got := includesMostFoldableRunes(re); got != tt.want {
+			t.Errorf("includesMostFoldableRunes(%#q) = %t, want %t", tt.regexp, got, tt.want)
+		}
+	}
+}
+
+func TestFoldSensitive(t *testing.T) {
+	for _, tt := range []struct {
+		regexp string
+		want   bool
+	}{
+		{`\d`, false},   // no foldables -> safe to use ?i
+		{`[A-B]`, true}, // doesn't have lowercase -> unsafe
+		{`[kK]`, true},  // doesn't have Kelvin rune
+		{`[A-Ba-b]`, false},
+		{`\W`, true},             // some non-word runes fold to A-Za-z
+		{`[A-\x{1e943}]`, false}, // entire foldable range
+		{`\D`, false},            // excluded runes (digits) don't fold
+	} {
+		fmt.Println(tt.regexp) // XXX DEBUG
+		re, err := Parse(tt.regexp, MatchNL|Perl&^OneLine)
+		if err != nil {
+			t.Errorf("Parse(%#q) = error %v", tt.regexp, err)
+			continue
+		}
+		if got := foldSensitive(re); got != tt.want {
+			t.Errorf("foldSensitive(%#q) = %t, want %t", tt.regexp, got, tt.want)
+		}
+	}
+}
diff --git a/src/regexp/syntax/simplify_test.go b/src/regexp/syntax/simplify_test.go
index 6d06f99..c84b8e9 100644
--- a/src/regexp/syntax/simplify_test.go
+++ b/src/regexp/syntax/simplify_test.go
@@ -114,6 +114,10 @@
 	{`(?i)[a-z]`, "[A-Za-z\u017F\u212A]"},
 	{`(?i)[\x00-\x{FFFD}]`, "[\\x00-\uFFFD]"},
 	{`(?i)[\x00-\x{10FFFF}]`, `(?s:.)`},
+	{`[Aa][Bb][Kk]`, `(?i:AB)[Kk]`},
+	{`[Aa][Bb][KkK]`, `(?i:AB[KkK])`},
+	{`[Aa][Bb][O-u]`, `(?i:AB)[O-u]`},
+	{`(?i)[0-24-6A]`, `[0-24-6Aa]`},
 
 	// Empty string as a regular expression.
 	// The empty string must be preserved inside parens in order

Change information

Files:
  • M src/regexp/syntax/regexp.go
  • A src/regexp/syntax/regexp_test.go
  • M src/regexp/syntax/simplify_test.go
Change size: M
Delta: 3 files changed, 157 insertions(+), 12 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
unsatisfied_requirement
satisfied_requirement
open
diffy

Gopher Robot (Gerrit)

unread,
Jul 10, 2026, 7:21:07 PM (7 days ago) Jul 10
to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Message from Gopher Robot

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Comment-Date: Fri, 10 Jul 2026 23:21:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
unsatisfied_requirement
satisfied_requirement
open
diffy

Caleb Spare (Gerrit)

unread,
Jul 10, 2026, 7:28:12 PM (7 days ago) Jul 10
to Nicholas Feinberg, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Feinberg

Caleb Spare voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Feinberg
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Comment-Date: Fri, 10 Jul 2026 23:28:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
unsatisfied_requirement
satisfied_requirement
open
diffy

Caleb Spare (Gerrit)

unread,
Jul 11, 2026, 2:02:59 AM (7 days ago) Jul 11
to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Feinberg

Caleb Spare voted and added 12 comments

Votes added by Caleb Spare

Code-Review+2

12 comments

Commit Message
Line 7, Patchset 1 (Latest):regexp: fix pathological slowdown in String for negated classes
Caleb Spare . unresolved

This should be regexp/syntax, not regexp.

Line 9, Patchset 1 (Latest):The existing implementation of regexp.String() is very inefficient
Caleb Spare . unresolved

I'd just write this as "String" (and no "()" -- that's the standard Go style).

It's not regexp.String, it's really (*syntax.Regexp).String. But String is probably understandable enough.

Line 12, Patchset 1 (Latest):case-insensitive ?i flag to such classes, calcFlags() iterates
Caleb Spare . unresolved

s/()//

File src/regexp/syntax/regexp.go
Line 218, Patchset 1 (Latest): for i := range len(re.Rune) / 2 {
Caleb Spare . unresolved

Throughout, let's stick with iterating by 2 as the original code did rather than the `i*2` thing.

Line 219, Patchset 1 (Latest): lowRune := re.Rune[i*2]
Caleb Spare . unresolved

Nit: throughout, instead of `lowRune`/`highRune` or `low`/`high`, how about just `lo`/`hi` -- it's a common naming choice that doesn't lose any clarity and it's what the code you're editing already used.

Line 241, Patchset 1 (Latest):func foldSensitive(re *Regexp) bool {
Caleb Spare . unresolved

Nit: let's reverse the order of these three functions. That puts the more important function first and its helper after.

Line 242, Patchset 1 (Latest): // Being fold-sensitive means that some rune within one of the regexp's

// ranges has a fold variant which is not in any of its ranges.
Caleb Spare . unresolved

I'd rewrite this as a doc comment.

Also, since this is now far away from where it's called, might be worth mentioning that it's specific to a char class node (or even rename this function to mention it).

Line 245, Patchset 1 (Latest): // Many common expressions, such as \W, include a huge range of runes,
Caleb Spare . unresolved

Maybe s/expressions/classes/ would be more precise?

Line 248, Patchset 1 (Latest): // loop over runes which are *not* in the regexp, and check if *they*

// have a fold variant which *is* in the regexp (a symmetric property).
Caleb Spare . unresolved

I think s/regexp/class/ both times.

Line 250, Patchset 1 (Latest): if !includesMostFoldableRunes(re) {
Caleb Spare . unresolved

If I'm reading it correctly, the big comment which precedes this is mostly about how to handle the `if includesMostFoldableRunes(re)` case, which is further down the page. I think it would make more sense to put that one first, both to get rid of the `!` here and to make the comment better align with the code.

File src/regexp/syntax/regexp_test.go
Line 68, Patchset 1 (Latest): fmt.Println(tt.regexp) // XXX DEBUG
Caleb Spare . unresolved

Delete

File src/regexp/syntax/simplify_test.go
File-level comment, Patchset 1 (Latest):
Caleb Spare . unresolved

No actual test would fail without this change, right?

Should we add a new benchmark, at least?

Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Feinberg
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Comment-Date: Sat, 11 Jul 2026 06:02:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Caleb Spare (Gerrit)

unread,
Jul 11, 2026, 2:03:07 AM (7 days ago) Jul 11
to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Feinberg

Caleb Spare voted Code-Review+0

Code-Review+0
Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Feinberg
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Comment-Date: Sat, 11 Jul 2026 06:03:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
unsatisfied_requirement
satisfied_requirement
open
diffy

Caleb Spare (Gerrit)

unread,
Jul 11, 2026, 2:04:25 AM (7 days ago) Jul 11
to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Feinberg

Caleb Spare added 1 comment

Patchset-level comments
Caleb Spare . resolved

Whoops, didn't realize that the big +2 button would just submit my comments and approve without any further confirmation.

Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Feinberg
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
Gerrit-Change-Number: 799641
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
Gerrit-Comment-Date: Sat, 11 Jul 2026 06:04:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
satisfied_requirement
open
diffy

Nicholas Feinberg (Gerrit)

unread,
Jul 13, 2026, 5:00:17 PM (4 days ago) Jul 13
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Nicholas Feinberg

Nicholas Feinberg uploaded new patchset

Nicholas Feinberg uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Feinberg
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
    Gerrit-Change-Number: 799641
    Gerrit-PatchSet: 2
    unsatisfied_requirement
    open
    diffy

    Nicholas Feinberg (Gerrit)

    unread,
    Jul 13, 2026, 5:02:49 PM (4 days ago) Jul 13
    to goph...@pubsubhelper.golang.org, Caleb Spare, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Caleb Spare

    Nicholas Feinberg added 13 comments

    Patchset-level comments
    File-level comment, Patchset 1:
    Nicholas Feinberg . resolved

    I believe I've handled all requested changes. Thanks for the review!

    Ran a quick benchmark on my laptop - not very precise, but should be good enough for a change like this :)

    ```
    goos: darwin
    goarch: arm64
    pkg: std/regexp/syntax
    cpu: Apple M5 Max
              │     old.txt     │               new.txt               │
              │     sec/op      │   sec/op     vs base                │
    String-18   1115880.0n ± 3%   379.2n ± 6%  -99.97% (p=0.000 n=10)
    ```
    Commit Message
    Line 7, Patchset 1:regexp: fix pathological slowdown in String for negated classes
    Caleb Spare . resolved

    This should be regexp/syntax, not regexp.

    Nicholas Feinberg

    Done

    Line 9, Patchset 1:The existing implementation of regexp.String() is very inefficient
    Caleb Spare . resolved

    I'd just write this as "String" (and no "()" -- that's the standard Go style).

    It's not regexp.String, it's really (*syntax.Regexp).String. But String is probably understandable enough.

    Nicholas Feinberg

    Done

    Line 12, Patchset 1:case-insensitive ?i flag to such classes, calcFlags() iterates
    Caleb Spare . resolved

    s/()//

    Nicholas Feinberg

    Done

    File src/regexp/syntax/regexp.go
    Line 218, Patchset 1: for i := range len(re.Rune) / 2 {
    Caleb Spare . resolved

    Throughout, let's stick with iterating by 2 as the original code did rather than the `i*2` thing.

    Nicholas Feinberg

    Done

    Line 219, Patchset 1: lowRune := re.Rune[i*2]
    Caleb Spare . resolved

    Nit: throughout, instead of `lowRune`/`highRune` or `low`/`high`, how about just `lo`/`hi` -- it's a common naming choice that doesn't lose any clarity and it's what the code you're editing already used.

    Nicholas Feinberg

    Done

    Line 241, Patchset 1:func foldSensitive(re *Regexp) bool {
    Caleb Spare . resolved

    Nit: let's reverse the order of these three functions. That puts the more important function first and its helper after.

    Nicholas Feinberg

    Done

    Line 242, Patchset 1: // Being fold-sensitive means that some rune within one of the regexp's

    // ranges has a fold variant which is not in any of its ranges.
    Caleb Spare . resolved

    I'd rewrite this as a doc comment.

    Also, since this is now far away from where it's called, might be worth mentioning that it's specific to a char class node (or even rename this function to mention it).

    Nicholas Feinberg

    Done

    Line 245, Patchset 1: // Many common expressions, such as \W, include a huge range of runes,
    Caleb Spare . resolved

    Maybe s/expressions/classes/ would be more precise?

    Nicholas Feinberg

    Done

    Line 248, Patchset 1: // loop over runes which are *not* in the regexp, and check if *they*

    // have a fold variant which *is* in the regexp (a symmetric property).
    Caleb Spare . resolved

    I think s/regexp/class/ both times.

    Nicholas Feinberg

    Done

    Line 250, Patchset 1: if !includesMostFoldableRunes(re) {
    Caleb Spare . resolved

    If I'm reading it correctly, the big comment which precedes this is mostly about how to handle the `if includesMostFoldableRunes(re)` case, which is further down the page. I think it would make more sense to put that one first, both to get rid of the `!` here and to make the comment better align with the code.

    Nicholas Feinberg

    Done

    File src/regexp/syntax/regexp_test.go
    Line 68, Patchset 1: fmt.Println(tt.regexp) // XXX DEBUG
    Caleb Spare . resolved

    Delete

    Nicholas Feinberg

    Done

    File src/regexp/syntax/simplify_test.go
    File-level comment, Patchset 1:
    Caleb Spare . resolved

    No actual test would fail without this change, right?

    Should we add a new benchmark, at least?

    Nicholas Feinberg

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Caleb Spare
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
      Gerrit-Change-Number: 799641
      Gerrit-PatchSet: 2
      Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
      Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
      Gerrit-CC: Gopher Robot <go...@golang.org>
      Gerrit-Attention: Caleb Spare <ces...@gmail.com>
      Gerrit-Comment-Date: Mon, 13 Jul 2026 21:02:43 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Caleb Spare <ces...@gmail.com>
      unsatisfied_requirement
      satisfied_requirement
      open
      diffy

      Caleb Spare (Gerrit)

      unread,
      Jul 13, 2026, 6:06:27 PM (4 days ago) Jul 13
      to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
      Attention needed from Nicholas Feinberg

      Caleb Spare voted and added 2 comments

      Votes added by Caleb Spare

      Code-Review+1
      Commit-Queue+1

      2 comments

      File src/regexp/syntax/regexp.go
      Line 216, Patchset 2 (Latest):// foldSensitiveCharClass returns, for a charClass node, whether some rune in
      Caleb Spare . unresolved

      s/returns/reports/

      File src/regexp/syntax/regexp_test.go
      Line 78, Patchset 2 (Latest):func BenchmarkString(b *testing.B) {
      Caleb Spare . unresolved

      Add a comment here linking to the issue (as `https://go.dev/issue/69303`) to explain the existence of this benchmark. Or maybe even rename it to `BenchmarkIssue69303` -- it's not a general-purpose benchmark for `String`, it checks for a regression in this specific issue.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Nicholas Feinberg
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
        Gerrit-Change-Number: 799641
        Gerrit-PatchSet: 2
        Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
        Gerrit-CC: Gopher Robot <go...@golang.org>
        Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Comment-Date: Mon, 13 Jul 2026 22:06:16 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        unsatisfied_requirement
        open
        diffy

        Caleb Spare (Gerrit)

        unread,
        Jul 13, 2026, 6:07:10 PM (4 days ago) Jul 13
        to Nicholas Feinberg, goph...@pubsubhelper.golang.org, Russ Cox, golang...@luci-project-accounts.iam.gserviceaccount.com, Gopher Robot, golang-co...@googlegroups.com
        Attention needed from Nicholas Feinberg and Russ Cox

        Caleb Spare added 1 comment

        Patchset-level comments
        File-level comment, Patchset 2 (Latest):
        Caleb Spare . resolved

        Adding rsc as the reviewer per s/owners.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Nicholas Feinberg
        • Russ Cox
        Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
        Gerrit-Change-Number: 799641
        Gerrit-PatchSet: 2
        Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
        Gerrit-Reviewer: Russ Cox <r...@golang.org>
        Gerrit-Attention: Russ Cox <r...@golang.org>
        Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Comment-Date: Mon, 13 Jul 2026 22:07:04 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        unsatisfied_requirement
        open
        diffy

        Nicholas Feinberg (Gerrit)

        unread,
        Jul 13, 2026, 6:27:28 PM (4 days ago) Jul 13
        to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
        Attention needed from Caleb Spare, Nicholas Feinberg and Russ Cox

        Nicholas Feinberg uploaded new patchset

        Nicholas Feinberg uploaded patch set #3 to this change.
        Following approvals got outdated and were removed:
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Caleb Spare
        • Nicholas Feinberg
        • Russ Cox
        Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: newpatchset
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
        Gerrit-Change-Number: 799641
        Gerrit-PatchSet: 3
        Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
        Gerrit-Reviewer: Russ Cox <r...@golang.org>
        Gerrit-CC: Gopher Robot <go...@golang.org>
        Gerrit-Attention: Russ Cox <r...@golang.org>
        Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
        Gerrit-Attention: Caleb Spare <ces...@gmail.com>
        unsatisfied_requirement
        open
        diffy

        Nicholas Feinberg (Gerrit)

        unread,
        Jul 13, 2026, 6:28:14 PM (4 days ago) Jul 13
        to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Russ Cox, Caleb Spare, Gopher Robot, golang-co...@googlegroups.com
        Attention needed from Caleb Spare and Russ Cox

        Nicholas Feinberg added 2 comments

        File src/regexp/syntax/regexp.go
        Line 216, Patchset 2:// foldSensitiveCharClass returns, for a charClass node, whether some rune in
        Caleb Spare . resolved

        s/returns/reports/

        Nicholas Feinberg

        Done

        File src/regexp/syntax/regexp_test.go
        Line 78, Patchset 2:func BenchmarkString(b *testing.B) {
        Caleb Spare . resolved

        Add a comment here linking to the issue (as `https://go.dev/issue/69303`) to explain the existence of this benchmark. Or maybe even rename it to `BenchmarkIssue69303` -- it's not a general-purpose benchmark for `String`, it checks for a regression in this specific issue.

        Nicholas Feinberg

        Done

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Caleb Spare
        • Russ Cox
        Submit Requirements:
          • requirement is not satisfiedCode-Review
          • requirement satisfiedNo-Unresolved-Comments
          • requirement is not satisfiedReview-Enforcement
          • requirement is not satisfiedTryBots-Pass
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
          Gerrit-Change-Number: 799641
          Gerrit-PatchSet: 3
          Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
          Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
          Gerrit-Reviewer: Russ Cox <r...@golang.org>
          Gerrit-CC: Gopher Robot <go...@golang.org>
          Gerrit-Attention: Russ Cox <r...@golang.org>
          Gerrit-Attention: Caleb Spare <ces...@gmail.com>
          Gerrit-Comment-Date: Mon, 13 Jul 2026 22:28:10 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          Comment-In-Reply-To: Caleb Spare <ces...@gmail.com>
          unsatisfied_requirement
          satisfied_requirement
          open
          diffy

          Caleb Spare (Gerrit)

          unread,
          6:44 PM (5 hours ago) 6:44 PM
          to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
          Attention needed from Nicholas Feinberg and Russ Cox

          Caleb Spare added 1 comment

          File src/regexp/syntax/regexp_test.go
          Line 1, Patchset 3 (Latest):// Copyright 2024 The Go Authors. All rights reserved.
          Caleb Spare . unresolved

          2026

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Nicholas Feinberg
          • Russ Cox
          Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
            Gerrit-Change-Number: 799641
            Gerrit-PatchSet: 3
            Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
            Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
            Gerrit-Reviewer: Russ Cox <r...@golang.org>
            Gerrit-CC: Gopher Robot <go...@golang.org>
            Gerrit-Attention: Russ Cox <r...@golang.org>
            Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
            Gerrit-Comment-Date: Fri, 17 Jul 2026 22:44:05 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            unsatisfied_requirement
            open
            diffy

            Nicholas Feinberg (Gerrit)

            unread,
            6:49 PM (4 hours ago) 6:49 PM
            to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
            Attention needed from Nicholas Feinberg and Russ Cox

            Nicholas Feinberg uploaded new patchset

            Nicholas Feinberg uploaded patch set #4 to this change.
            Open in Gerrit

            Related details

            Attention is currently required from:
            • Nicholas Feinberg
            • Russ Cox
            Submit Requirements:
            • requirement is not satisfiedCode-Review
            • requirement is not satisfiedNo-Unresolved-Comments
            • requirement is not satisfiedReview-Enforcement
            • requirement is not satisfiedTryBots-Pass
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: newpatchset
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
            Gerrit-Change-Number: 799641
            Gerrit-PatchSet: 4
            unsatisfied_requirement
            open
            diffy

            Nicholas Feinberg (Gerrit)

            unread,
            6:50 PM (4 hours ago) 6:50 PM
            to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Russ Cox, Caleb Spare, Gopher Robot, golang-co...@googlegroups.com
            Attention needed from Caleb Spare and Russ Cox

            Nicholas Feinberg added 1 comment

            File src/regexp/syntax/regexp_test.go
            Line 1, Patchset 3:// Copyright 2024 The Go Authors. All rights reserved.
            Caleb Spare . resolved

            2026

            Nicholas Feinberg

            Done

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Caleb Spare
            • Russ Cox
            Submit Requirements:
              • requirement is not satisfiedCode-Review
              • requirement satisfiedNo-Unresolved-Comments
              • requirement is not satisfiedReview-Enforcement
              • requirement is not satisfiedTryBots-Pass
              Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
              Gerrit-MessageType: comment
              Gerrit-Project: go
              Gerrit-Branch: master
              Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
              Gerrit-Change-Number: 799641
              Gerrit-PatchSet: 4
              Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
              Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
              Gerrit-Reviewer: Russ Cox <r...@golang.org>
              Gerrit-CC: Gopher Robot <go...@golang.org>
              Gerrit-Attention: Russ Cox <r...@golang.org>
              Gerrit-Attention: Caleb Spare <ces...@gmail.com>
              Gerrit-Comment-Date: Fri, 17 Jul 2026 22:49:59 +0000
              Gerrit-HasComments: Yes
              Gerrit-Has-Labels: No
              Comment-In-Reply-To: Caleb Spare <ces...@gmail.com>
              unsatisfied_requirement
              satisfied_requirement
              open
              diffy

              Caleb Spare (Gerrit)

              unread,
              6:56 PM (4 hours ago) 6:56 PM
              to Nicholas Feinberg, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
              Attention needed from Nicholas Feinberg and Russ Cox

              Caleb Spare voted

              Code-Review+1
              Commit-Queue+1
              Open in Gerrit

              Related details

              Attention is currently required from:
              • Nicholas Feinberg
              • Russ Cox
              Submit Requirements:
              • requirement is not satisfiedCode-Review
              • requirement satisfiedNo-Unresolved-Comments
              • requirement is not satisfiedReview-Enforcement
              • requirement is not satisfiedTryBots-Pass
              Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
              Gerrit-MessageType: comment
              Gerrit-Project: go
              Gerrit-Branch: master
              Gerrit-Change-Id: I5cca20d2453e720d7b11973e7c646402b1337334
              Gerrit-Change-Number: 799641
              Gerrit-PatchSet: 4
              Gerrit-Owner: Nicholas Feinberg <nich...@liftoff.io>
              Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
              Gerrit-Reviewer: Russ Cox <r...@golang.org>
              Gerrit-CC: Gopher Robot <go...@golang.org>
              Gerrit-Attention: Russ Cox <r...@golang.org>
              Gerrit-Attention: Nicholas Feinberg <nich...@liftoff.io>
              Gerrit-Comment-Date: Fri, 17 Jul 2026 22:56:30 +0000
              Gerrit-HasComments: No
              Gerrit-Has-Labels: Yes
              unsatisfied_requirement
              satisfied_requirement
              open
              diffy
              Reply all
              Reply to author
              Forward
              0 new messages