Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Avoid recompiling regexes #1293

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: Avoid recompiling regexes #1293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 11 additions & 25 deletions build/rewrite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ func keepSorted(x Expr) bool {
return hasComment(x, "keep sorted")
}

// labelRE matches label strings, e.g. @r//x/y/z:abc
// where $1 is @r//x/y/z, $2 is @r//, $3 is r, $4 is z, $5 is abc.
func makeLabelRe(labelPrefix string) *regexp.Regexp {
return regexp.MustCompile(`^(((?:@(\w+))?//|` + labelPrefix + `)(?:.+/)?([^:]*))(?::([^:]+))?$`)
}

var leadingSlashesLabelRe = makeLabelRe("//")
var stripLeadingSlashesLabelRe = makeLabelRe("")

// fixLabels rewrites labels into a canonical form.
//
// First, it joins labels written as string addition, turning
Expand Down Expand Up @@ -248,12 +257,11 @@ func fixLabels(f *File, w *Rewriter) {
}

labelPrefix := "//"
labelRE := leadingSlashesLabelRe
if w.StripLabelLeadingSlashes {
labelPrefix = ""
labelRE = stripLeadingSlashesLabelRe
}
// labelRE matches label strings, e.g. @r//x/y/z:abc
// where $1 is @r//x/y/z, $2 is @r//, $3 is r, $4 is z, $5 is abc.
labelRE := regexp.MustCompile(`^(((?:@(\w+))?//|` + labelPrefix + `)(?:.+/)?([^:]*))(?::([^:]+))?$`)

shortenLabel := func(v Expr) {
str, ok := v.(*StringExpr)
Expand Down Expand Up @@ -699,28 +707,6 @@ func isUniq(list []stringSortKey) bool {
return true
}

// If stk describes a call argument like rule(arg=...), callArgName
// returns the name of that argument, formatted as "rule.arg".
func callArgName(stk []Expr) string {
n := len(stk)
if n < 2 {
return ""
}
arg := argName(stk[n-1])
if arg == "" {
return ""
}
call, ok := stk[n-2].(*CallExpr)
if !ok {
return ""
}
rule, ok := call.X.(*Ident)
if !ok {
return ""
}
return rule.Name + "." + arg
}

// A stringSortKey records information about a single string literal to be
// sorted. The strings are first grouped into four phases: most strings,
// strings beginning with ":", strings beginning with "//", and strings
Expand Down
10 changes: 6 additions & 4 deletions edit/fix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,14 @@ func genruleRenameDepsTools(_ *build.File, r *build.Rule, _ string) bool {
return r.Kind() == "genrule" && RenameAttribute(r, "deps", "tools") == nil
}

// Regexp comes from LABEL_CHAR_MATCHER in
//
// java/com/google/devtools/build/lib/analysis/LabelExpander.java
var labelCharMatcherRe = regexp.MustCompile("[a-zA-Z0-9:/_.+-]+|[^a-zA-Z0-9:/_.+-]+")

// explicitHeuristicLabels adds $(location ...) for each label in the string s.
func explicitHeuristicLabels(s string, labels map[string]bool) string {
// Regexp comes from LABEL_CHAR_MATCHER in
// java/com/google/devtools/build/lib/analysis/LabelExpander.java
re := regexp.MustCompile("[a-zA-Z0-9:/_.+-]+|[^a-zA-Z0-9:/_.+-]+")
parts := re.FindAllString(s, -1)
parts := labelCharMatcherRe.FindAllString(s, -1)
changed := false
canChange := true
for i, part := range parts {
Expand Down
4 changes: 2 additions & 2 deletions warn/warn_cosmetic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ func unsortedDictItemsWarning(f *build.File) []*LinterFinding {
"Dictionary items are out of their lexicographical order.",
LinterReplacement{expr, &newDict}))
}
return
})
return findings
}
Expand All @@ -216,14 +215,15 @@ func skylarkToStarlark(s string) string {
}
}

var skylarkRegex = regexp.MustCompile("(?i)skylark")

// replaceSkylark replaces a substring "skylark" (case-insensitive) with a
// similar cased string "starlark". Doesn't replace it if the previous or the
// next symbol is '/', which may indicate it's a part of a URL.
// Normally that should be done with look-ahead and look-behind assertions in a
// regular expression, but negative look-aheads and look-behinds are not
// supported by Go regexp module.
func replaceSkylark(s string) (newString string, changed bool) {
skylarkRegex := regexp.MustCompile("(?i)skylark")
newString = s
for _, r := range skylarkRegex.FindAllStringIndex(s, -1) {
if r[0] > 0 && s[r[0]-1] == '/' {
Expand Down