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.

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

Avoid copying loop vars (Go 1.22+) #7191

Merged
merged 1 commit into from
Nov 24, 2024
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
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ linters:
- gosimple
- prealloc
- unconvert
- copyloopvar
# - gosec # too many false positives
2 changes: 0 additions & 2 deletions cmd/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,6 @@ a[4] {

for _, mode := range modes {
for _, tc := range tests {
tc := tc // Reassignment prevents loop var scoping issue. (See: https://go.dev/blog/loopvar-preview)
t.Run(fmt.Sprintf("%s, %s", tc.note, mode.name), func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1023,7 +1022,6 @@ a contains 4 if {
for _, bundleType := range bundleTypeCases {
for _, mode := range modes {
for _, tc := range tests {
tc := tc // Reassignment prevents loop var scoping issue. (See: https://go.dev/blog/loopvar-preview)
t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, tc.note, mode.name), func(t *testing.T) {
t.Parallel()

Expand Down
1 change: 0 additions & 1 deletion cmd/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,6 @@ func evalOnce(ctx context.Context, ectx *evalContext) pr.Output {
result.Errors = pr.NewOutputErrors(resultErr)
if ectx.builtInErrorList != nil {
for _, err := range *(ectx.builtInErrorList) {
err := err
result.Errors = append(result.Errors, pr.NewOutputErrors(&err)...)
}
}
Expand Down
1 change: 0 additions & 1 deletion debug/debugger.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ func (s *session) start() error {
defer s.mtx.Unlock()

for _, t := range s.threads {
t := t
go func() {
s.d.logger.Debug("Thread %d started", t.id)
s.d.sendEvent(Event{Type: ThreadEventType, Thread: t.id, Message: "started"})
Expand Down
1 change: 0 additions & 1 deletion storage/disk/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ func TestBadgerConfigFromOptions(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down
5 changes: 0 additions & 5 deletions storage/disk/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,6 @@ func TestDataPartitioningReadsAndWrites(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1301,7 +1300,6 @@ func TestDataPartitioningReadNotFoundErrors(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1458,7 +1456,6 @@ func TestDataPartitioningWriteNotFoundErrors(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1507,7 +1504,6 @@ func TestDataPartitioningWriteInvalidPatchError(t *testing.T) {
t.Parallel()

for _, pt := range []string{"/*", "/foo"} {
pt := pt // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(pt, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1672,7 +1668,6 @@ func TestLookup(t *testing.T) {
}

for _, tc := range cases {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down
1 change: 0 additions & 1 deletion storage/disk/partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ func TestPartitionTrie(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(strings.TrimPrefix(tc.path, "/"), func(t *testing.T) {
t.Parallel()

Expand Down
1 change: 0 additions & 1 deletion storage/disk/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func TestIsDisjoint(t *testing.T) {
overlapped: true,
},
} {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down
7 changes: 3 additions & 4 deletions topdown/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,6 @@ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
},
}
for name, testData := range tests {
testData := testData // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(name, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -829,9 +828,9 @@ func TestExtractX509VerifyOptions(t *testing.T) {
},
},
{ // KeyUsages as an array
jsonOption: ast.MustParseTerm(`{"DNSName": "test.com", "CurrentTime": 1708447636000000000,
"MaxConstraintComparisons": 5,
"KeyUsages" : ["KeyUsageAny", "KeyUsageAny", 1, 2,
jsonOption: ast.MustParseTerm(`{"DNSName": "test.com", "CurrentTime": 1708447636000000000,
"MaxConstraintComparisons": 5,
"KeyUsages" : ["KeyUsageAny", "KeyUsageAny", 1, 2,
"KeyUsageServerAuth","KeyUsageClientAuth"]}`),
expectVerifyOpt: x509.VerifyOptions{
DNSName: "test.com",
Expand Down
1 change: 0 additions & 1 deletion topdown/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ func TestErrorWrapping(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down
67 changes: 31 additions & 36 deletions topdown/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ func TestMergeWhenHittingNonObject(t *testing.T) {
}

for _, tc := range cases {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -172,7 +171,6 @@ func TestRefContainsNonScalar(t *testing.T) {
}

for _, tc := range cases {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -252,7 +250,6 @@ func TestContainsNestedRefOrCall(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -586,13 +583,13 @@ func TestTopdownVirtualCache(t *testing.T) {
{
note: "partial object, ref-head, ref with unification scope",
module: `package test

a[x][y][z] := x + y + z if {
some x in [1, 2]
some y in [3, 4]
some z in [5, 6]
}

p if {
x := a[1][_][5] # miss, cache key: data.test.a[1][<_,5>]
some foo
Expand All @@ -606,14 +603,14 @@ func TestTopdownVirtualCache(t *testing.T) {
{
note: "partial object, ref-head, ref with unification scope, component order",
module: `package test

a[x][y][a][b] := i if {
some x in [1, 2]
some y in [3, 4]
some a in ["foo", "bar"]
some i, b in ["foo", "bar"]
}

p if {
x := a[1][_]["foo"]["bar"] # miss, cache key: data.test.a[1][<_,foo,bar>]
y := a[1][_]["bar"]["foo"] # miss, cache key: data.test.a[1][<_,bar,foo>]
Expand All @@ -626,13 +623,13 @@ func TestTopdownVirtualCache(t *testing.T) {
{
note: "partial object, ref-head, ref with unification scope, diverging key scope",
module: `package test

a[x][y][z] := x + y + z if {
some x in [1, 2]
some y in [3, 4]
some z in [5, 6]
}

p if {
x := a[1][_][5] # miss, cache key: data.test.a[1][<_,5>]
y := a[1][_][6] # miss, cache key: data.test.a[1][<_,6>]
Expand All @@ -647,13 +644,13 @@ func TestTopdownVirtualCache(t *testing.T) {
{
note: "partial object, ref-head, ref with unification scope, trailing vars don't contribute to key scope",
module: `package test

a[x][y][z][x] := x + y + z if {
some x in [1, 2]
some y in [3, 4]
some z in [5, 6]
}

p if {
x := a[1][_][5][_] # miss, cache key: data.test.a[1][<_,5>]
y := a[1][_][5] # hit, cache key: data.test.a[1][<_,5>]
Expand All @@ -667,11 +664,11 @@ func TestTopdownVirtualCache(t *testing.T) {
// Regression test for https://github.com/open-policy-agent/opa/issues/6926
note: "partial object, ref-head, leaf set, ref with unification scope",
module: `package p

obj.sub[x][x] contains x if some x in ["one", "two"]

obj[x][x] contains x if x := "whatever"

main contains x if {
[1 | obj.sub[_].one[_]] # miss, cache key: data.p.obj.sub[<_,one>]
x := obj.sub[_][_][_] # miss, cache key: data.p.obj.sub
Expand All @@ -683,7 +680,6 @@ func TestTopdownVirtualCache(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -988,9 +984,9 @@ func TestPartialRule(t *testing.T) {
`,
query: `data = x`,
exp: `[{"x": {"test": {"p": {"foo": {
"0": {"bar": "a"},
"1": {"bar": "b"},
"2": {"bar": "c"},
"0": {"bar": "a"},
"1": {"bar": "b"},
"2": {"bar": "c"},
"bar": {"0": "a", "1": "b", "2": "c"}}}}}}]`,
},
// Intersections with object values
Expand Down Expand Up @@ -1131,8 +1127,8 @@ func TestPartialRule(t *testing.T) {
{
note: "deep query into partial object (ref head) and object value",
module: `package test
p.q[r] := x if {
r := "foo"
p.q[r] := x if {
r := "foo"
x := {"bar": {"baz": 1}}
}
`,
Expand Down Expand Up @@ -1258,7 +1254,7 @@ func TestPartialRule(t *testing.T) {
{ // enumeration
note: "deep query into partial object and object value, full depth, enumeration on object value",
module: `package test
p.q[r] := x if {
p.q[r] := x if {
r := ["foo", "bar"][_]
x := {"s": {"do": 0, "re": 1, "mi": 2}}
}
Expand All @@ -1269,7 +1265,7 @@ func TestPartialRule(t *testing.T) {
{ // enumeration
note: "deep query into partial object and object value, full depth, enumeration on rule path and object value",
module: `package test
p.q[r] := x if {
p.q[r] := x if {
r := ["foo", "bar"][_]
x := {"s": {"do": 0, "re": 1, "mi": 2}}
}
Expand All @@ -1292,8 +1288,8 @@ func TestPartialRule(t *testing.T) {
note: "deep query into partial object (general ref head) and set value",
module: `package test
import future.keywords
p.q[r] contains t if {
r := ["foo", "bar"][_]
p.q[r] contains t if {
r := ["foo", "bar"][_]
{"do", "re", "mi"}[t]
}
`,
Expand All @@ -1304,8 +1300,8 @@ func TestPartialRule(t *testing.T) {
note: "deep query into partial object (general ref head, static tail) and set value",
module: `package test
import future.keywords
p.q[r].s contains t if {
r := ["foo", "bar"][_]
p.q[r].s contains t if {
r := ["foo", "bar"][_]
{"do", "re", "mi"}[t]
}
`,
Expand All @@ -1316,8 +1312,8 @@ func TestPartialRule(t *testing.T) {
note: "deep query into general ref to set value",
module: `package test
import future.keywords
p.q[r].s contains t if {
r := ["foo", "bar"][_]
p.q[r].s contains t if {
r := ["foo", "bar"][_]
t := ["do", "re", "mi"][_]
}
`,
Expand All @@ -1327,8 +1323,8 @@ func TestPartialRule(t *testing.T) {
{
note: "deep query into general ref to object value",
module: `package test
p.q[r].s[t] := u if {
r := ["foo", "bar"][_]
p.q[r].s[t] := u if {
r := ["foo", "bar"][_]
t := ["do", "re", "mi"][u]
}
`,
Expand All @@ -1339,8 +1335,8 @@ func TestPartialRule(t *testing.T) {
note: "deep query into general ref enumerating set values",
module: `package test
import future.keywords
p.q[r].s contains t if {
r := ["foo", "bar"][_]
p.q[r].s contains t if {
r := ["foo", "bar"][_]
{"do", "re", "mi"}[t]
}
`,
Expand All @@ -1351,8 +1347,8 @@ func TestPartialRule(t *testing.T) {
{
note: "deep query into partial object and object value, non-tail var",
module: `package test
p.q[r].s := x if {
r := "foo"
p.q[r].s := x if {
r := "foo"
x := {"bar": {"baz": 1}}
}
`,
Expand Down Expand Up @@ -1470,7 +1466,7 @@ func TestPartialRule(t *testing.T) {
t := ["d", "e", "f"][u]
}`,
query: `data.test.p.q[x] = y`,
exp: `[{"x": "a", "y": {"s": {"d": 0, "e": 1, "f": 2}}},
exp: `[{"x": "a", "y": {"s": {"d": 0, "e": 1, "f": 2}}},
{"x": "b", "y": {"s": {"d": 0, "e": 1, "f": 2}}},
{"x": "c", "y": {"s": {"d": 0, "e": 1, "f": 2}}}]`,
},
Expand Down Expand Up @@ -1535,7 +1531,6 @@ func TestPartialRule(t *testing.T) {
}

for _, tc := range tests {
tc := tc // copy for capturing loop variable (not needed in Go 1.22+)
t.Run(tc.note, func(t *testing.T) {
t.Parallel()

Expand Down
Loading