Skip to content

Commit 1224a7d

Browse files
findleyrawly
authored andcommitted
go/types, types2: do not mutate arguments in NewChecker
CL 507975 resulted in new data races (as reported in golang#61212), because the pkg argument to NewChecker was mutated. Fix this by deferring the recording of the goVersion in pkg until type checking is actually initiated via a call to Checker.Files. Additionally, modify types2/check.go to bring it in sync with the changes in go/types/check.go, and generate the new version_test.go from the types2 file. Also move parsing the version into checkFiles, for simplicity. Fixes golang#61212 Change-Id: I15edb6c2cff3085622fe7c6a3b0dab531d27bd04 Reviewed-on: https://go-review.googlesource.com/c/go/+/508439 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 55ed560 commit 1224a7d

File tree

4 files changed

+49
-0
lines changed

4 files changed

+49
-0
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2023 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package types2
6+
7+
import "testing"
8+
9+
var parseGoVersionTests = []struct {
10+
in string
11+
out version
12+
}{
13+
{"go1.21", version{1, 21}},
14+
{"go1.21.0", version{1, 21}},
15+
{"go1.21rc2", version{1, 21}},
16+
}
17+
18+
func TestParseGoVersion(t *testing.T) {
19+
for _, tt := range parseGoVersionTests {
20+
if out, err := parseGoVersion(tt.in); out != tt.out || err != nil {
21+
t.Errorf("parseGoVersion(%q) = %v, %v, want %v, nil", tt.in, out, err, tt.out)
22+
}
23+
}
24+
}

src/go/types/check.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,11 @@ type Checker struct {
109109
fset *token.FileSet
110110
pkg *Package
111111
*Info
112+
<<<<<<< HEAD
112113
version goVersion // accepted language version
114+
=======
115+
version version // accepted language version
116+
>>>>>>> a87a31d520 (go/types, types2: do not mutate arguments in NewChecker)
113117
nextID uint64 // unique Id for type parameters (first valid Id is 1)
114118
objMap map[Object]*declInfo // maps package-level objects and (non-interface) methods to declaration info
115119
impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
@@ -258,6 +262,7 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Ch
258262
// (previously, pkg.goVersion was mutated here: go.dev/issue/61212)
259263

260264
return &Checker{
265+
<<<<<<< HEAD
261266
enableAlias: gotypesalias.Value() == "1",
262267
conf: conf,
263268
ctxt: conf.Context,
@@ -267,6 +272,15 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Ch
267272
version: asGoVersion(conf.GoVersion),
268273
objMap: make(map[Object]*declInfo),
269274
impMap: make(map[importKey]*Package),
275+
=======
276+
conf: conf,
277+
ctxt: conf.Context,
278+
fset: fset,
279+
pkg: pkg,
280+
Info: info,
281+
objMap: make(map[Object]*declInfo),
282+
impMap: make(map[importKey]*Package),
283+
>>>>>>> a87a31d520 (go/types, types2: do not mutate arguments in NewChecker)
270284
}
271285
}
272286

@@ -379,6 +393,14 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
379393
// not be mutated. See https://go.dev/issue/61212 for an example of where
380394
// Unsafe is passed to NewChecker.
381395
return nil
396+
<<<<<<< HEAD
397+
=======
398+
}
399+
400+
check.version, err = parseGoVersion(check.conf.GoVersion)
401+
if err != nil {
402+
return err
403+
>>>>>>> a87a31d520 (go/types, types2: do not mutate arguments in NewChecker)
382404
}
383405

384406
// Note: NewChecker doesn't return an error, so we need to check the version here.

src/go/types/generate_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ var filemap = map[string]action{
143143
"universe.go": fixGlobalTypVarDecl,
144144
"util_test.go": fixTokenPos,
145145
"validtype.go": nil,
146+
"version_test.go": nil,
146147
}
147148

148149
// TODO(gri) We should be able to make these rewriters more configurable/composable.

src/go/types/version_test.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)