]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.typeparams] all: add GOEXPERIMENT=unified knob
authorMatthew Dempsky <mdempsky@google.com>
Thu, 17 Jun 2021 04:41:28 +0000 (21:41 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Thu, 17 Jun 2021 09:09:02 +0000 (09:09 +0000)
Setting `-gcflags=all=-d=unified` works for normal builds/tests, but
seems to have trouble with the test/run.go regress tests. So add a
GOEXPERIMENT knob to allow another way to turn on unified IR
construction, which plays better with all.bash.

While here, update two existing test expectations that currently fail
during GOEXPERIMENT=unified ./all.bash:

1. misc/cgo/errors/testdata/err2.go is testing column positions, and
types2 gets one case slightly better, and another case slightly
worse. For now, the test case is updated to accept both.

2. fixedbugs/issue42284.go is added to the list of known failures,
because it fails for unified IR. (It's an escape analysis test, and
escape analysis is working as expected; but unified is formatting an
imported constant value differently than the test's regexp expects.)

Updates #46786.

Change-Id: I40a4a70fa1b85ac87fcc85a43687f5d81e011ec0
Reviewed-on: https://go-review.googlesource.com/c/go/+/328215
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
misc/cgo/errors/testdata/err2.go
src/cmd/compile/internal/base/flag.go
src/internal/goexperiment/exp_unified_off.go [new file with mode: 0644]
src/internal/goexperiment/exp_unified_on.go [new file with mode: 0644]
src/internal/goexperiment/flags.go
test/run.go

index a90598fe35b6304434bba7253c29b1d62173dcc4..aa941584c3c2cfcaa7491f00733c4aa0244d674f 100644 (file)
@@ -91,10 +91,18 @@ func main() {
 
        // issue 26745
        _ = func(i int) int {
-               return C.i + 1 // ERROR HERE: 14
+               // typecheck reports at column 14 ('+'), but types2 reports at
+               // column 10 ('C').
+               // TODO(mdempsky): Investigate why, and see if types2 can be
+               // updated to match typecheck behavior.
+               return C.i + 1 // ERROR HERE: \b(10|14)\b
        }
        _ = func(i int) {
-               C.fi(i) // ERROR HERE: 7
+               // typecheck reports at column 7 ('('), but types2 reports at
+               // column 8 ('i'). The types2 position is more correct, but
+               // updating typecheck here is fundamentally challenging because of
+               // IR limitations.
+               C.fi(i) // ERROR HERE: \b(7|8)\b
        }
 
        C.fi = C.fi // ERROR HERE
index 42c0c1b94b559c17ae593b9592b7ec5152e3a5f8..b8b205f4127032f14abb8d1f3df39e5044ebe775 100644 (file)
@@ -159,7 +159,11 @@ func ParseFlags() {
        Flag.LinkShared = &Ctxt.Flag_linkshared
        Flag.Shared = &Ctxt.Flag_shared
        Flag.WB = true
+
        Debug.InlFuncsWithClosures = 1
+       if buildcfg.Experiment.Unified {
+               Debug.Unified = 1
+       }
 
        Debug.Checkptr = -1 // so we can tell whether it is set explicitly
 
diff --git a/src/internal/goexperiment/exp_unified_off.go b/src/internal/goexperiment/exp_unified_off.go
new file mode 100644 (file)
index 0000000..4c16fd8
--- /dev/null
@@ -0,0 +1,9 @@
+// Code generated by mkconsts.go. DO NOT EDIT.
+
+//go:build !goexperiment.unified
+// +build !goexperiment.unified
+
+package goexperiment
+
+const Unified = false
+const UnifiedInt = 0
diff --git a/src/internal/goexperiment/exp_unified_on.go b/src/internal/goexperiment/exp_unified_on.go
new file mode 100644 (file)
index 0000000..2b17ba3
--- /dev/null
@@ -0,0 +1,9 @@
+// Code generated by mkconsts.go. DO NOT EDIT.
+
+//go:build goexperiment.unified
+// +build goexperiment.unified
+
+package goexperiment
+
+const Unified = true
+const UnifiedInt = 1
index 71e38cd04795d35b73007219dffc8f96a56e23de..b7a62b3e26823644ffb5fe1f13586ae9e0735c30 100644 (file)
@@ -59,6 +59,10 @@ type Flags struct {
        PreemptibleLoops  bool
        StaticLockRanking bool
 
+       // Unified enables the compiler's unified IR construction
+       // experiment.
+       Unified bool
+
        // Regabi is split into several sub-experiments that can be
        // enabled individually. Not all combinations work.
        // The "regabi" GOEXPERIMENT is an alias for all "working"
index 656519e301722c9ed4c1e089d78775e43f83756b..f8bb8c081c787cc1363ee44bfdda1f59b81cc9b4 100644 (file)
@@ -58,8 +58,9 @@ func defaultAllCodeGen() bool {
 }
 
 var (
-       goos, goarch string
-       cgoEnabled   bool
+       goos, goarch   string
+       cgoEnabled     bool
+       unifiedEnabled bool
 
        // dirs are the directories to look for *.go files in.
        // TODO(bradfitz): just use all directories?
@@ -95,10 +96,27 @@ func main() {
 
        goos = getenv("GOOS", runtime.GOOS)
        goarch = getenv("GOARCH", runtime.GOARCH)
-       cgoEnv, err := exec.Command(goTool(), "env", "CGO_ENABLED").Output()
-       if err == nil {
-               cgoEnabled, _ = strconv.ParseBool(strings.TrimSpace(string(cgoEnv)))
+
+       cgoCmd := exec.Command(goTool(), "env", "CGO_ENABLED")
+       cgoEnv, err := cgoCmd.Output()
+       if err != nil {
+               log.Fatalf("running %v: %v", cgoCmd, err)
        }
+       cgoEnabled, _ = strconv.ParseBool(strings.TrimSpace(string(cgoEnv)))
+
+       // TODO(mdempsky): Change this to just "go env GOEXPERIMENT" after
+       // CL 328751 is merged back to dev.typeparams. In the mean time, we
+       // infer whether the "unified" experiment is defult enabled by
+       // inspecting the output from `go tool compile -V`.
+       compileCmd := exec.Command(goTool(), "tool", "compile", "-V")
+       compileOutput, err := compileCmd.Output()
+       if err != nil {
+               log.Fatalf("running %v: %v", compileCmd, err)
+       }
+       // TODO(mdempsky): This will give false negatives if the unified
+       // experiment is enabled by default, but presumably at that point we
+       // won't need to disable tests for it anymore anyway.
+       unifiedEnabled = strings.Contains(string(compileOutput), "unified")
 
        findExecCmd()
 
@@ -290,6 +308,10 @@ type test struct {
        err     error
 }
 
+// usesTypes2 reports whether the compiler uses types2 for this test
+// configuration (irrespective of flags specified by the test itself).
+func (t *test) usesTypes2() bool { return unifiedEnabled || t.glevel != 0 }
+
 func startTests(dir, gofile string, glevels []int) []*test {
        tests := make([]*test, len(glevels))
        for i, glevel := range glevels {
@@ -519,8 +541,8 @@ func (t *test) run() {
                close(t.donec)
        }()
 
-       if t.glevel > 0 && !*force {
-               // Files excluded from generics testing.
+       if t.usesTypes2() && !*force {
+               // Files excluded from types2 testing.
                filename := strings.Replace(t.goFileName(), "\\", "/", -1) // goFileName() uses \ on Windows
                if excludedFiles[filename] {
                        if *verbose {
@@ -666,19 +688,25 @@ func (t *test) run() {
        // at the specified -G level. If so, it may update flags as
        // necessary to test with -G.
        validForGLevel := func(tool Tool) bool {
-               if t.glevel == 0 {
-                       // default -G level; always valid
+               if !t.usesTypes2() {
+                       // tests should always pass when run w/o types2 (i.e., using the
+                       // legacy typechecker).
                        return true
                }
 
+               hasGFlag := false
                for _, flag := range flags {
                        if strings.Contains(flag, "-G") {
-                               // test provides explicit -G flag already
-                               if *verbose {
-                                       fmt.Printf("excl\t%s\n", t.goFileName())
-                               }
-                               return false
+                               hasGFlag = true
+                       }
+               }
+
+               if hasGFlag && t.glevel != 0 {
+                       // test provides explicit -G flag already; don't run again
+                       if *verbose {
+                               fmt.Printf("excl\t%s\n", t.goFileName())
                        }
+                       return false
                }
 
                switch tool {
@@ -686,7 +714,9 @@ func (t *test) run() {
                        // ok; handled in goGcflags
 
                case Compile:
-                       flags = append(flags, fmt.Sprintf("-G=%v", t.glevel))
+                       if !hasGFlag {
+                               flags = append(flags, fmt.Sprintf("-G=%v", t.glevel))
+                       }
 
                default:
                        // we don't know how to add -G for this test yet
@@ -2026,6 +2056,9 @@ func overlayDir(dstRoot, srcRoot string) error {
 
 // List of files that the compiler cannot errorcheck with the new typechecker (compiler -G option).
 // Temporary scaffolding until we pass all the tests at which point this map can be removed.
+//
+// TODO(mdempsky): Split exclude list to disambiguate whether the
+// failure is within types2, -G=3, or unified.
 var excludedFiles = map[string]bool{
        "directive.go":    true, // misplaced compiler directive checks
        "float_lit3.go":   true, // types2 reports extra errors
@@ -2079,10 +2112,11 @@ var excludedFiles = map[string]bool{
        "fixedbugs/issue33460.go":  true, // types2 reports alternative positions in separate error
        "fixedbugs/issue42058a.go": true, // types2 doesn't report "channel element type too large"
        "fixedbugs/issue42058b.go": true, // types2 doesn't report "channel element type too large"
-       "fixedbugs/issue46725.go":  true, // fix applied to typecheck needs to be ported to irgen/transform
+       "fixedbugs/issue42284.go":  true, // unified formats important constant expression differently in diagnostics
        "fixedbugs/issue4232.go":   true, // types2 reports (correct) extra errors
        "fixedbugs/issue4452.go":   true, // types2 reports (correct) extra errors
        "fixedbugs/issue4510.go":   true, // types2 reports different (but ok) line numbers
+       "fixedbugs/issue46725.go":  true, // fix applied to typecheck needs to be ported to irgen/transform
        "fixedbugs/issue5609.go":   true, // types2 needs a better error message
        "fixedbugs/issue7525b.go":  true, // types2 reports init cycle error on different line - ok otherwise
        "fixedbugs/issue7525c.go":  true, // types2 reports init cycle error on different line - ok otherwise