[dev.typeparams] test: fix and update run.go's generics testing

In a late change to golang.org/cl/320609, while going back and forth
on the meaning of the boolean result value for "checkFlags", I got one
of the cases wrong. As a result, rather than testing both default
flags and -G=3, we were (redundanly) testing default flags and -G=0.

I ran into this because in my local dev tree, I'm using types2 even
for -G=0, and evidently one of the recent types2 CLs changed the error
message in fixedbugs/issue10975.go. Fortunately, there haven't been
any other regressions despite lacking test coverage.

So this CL cleans things up a bit:

1. Fixes that test to use -lang=go1.17, so types2 reports the old
error message again.

2. Renames "checkFlags" to "validForGLevel" so the boolean result is
harder to get wrong.

3. Removes the blanket deny list of all -m tests, and instead adds the
specific tests that are still failing. This effectively extends -G=3
coverage to another 27 tests that were using -m but already passing,
so we can make sure they don't regress again.

4. Adds a -f flag to force running tests even if they're in the deny
list, to make it easier to test whether they're still failing without
having to edit run.go.

Change-Id: I058d9d90d81a92189e54c6f591d95fb617fede53
Reviewed-on: https://go-review.googlesource.com/c/go/+/322612
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

1 files changed, 70 insertions, 39 deletions

diff --git a/test/run.go b/test/run.go
index 506380a7a5..ef24396809 100644
--- a/test/run.go
+++ b/test/run.go
@@ -43,6 +43,7 @@ var (
 	updateErrors   = flag.Bool("update_errors", false, "update error messages in test file based on compiler output")
 	runoutputLimit = flag.Int("l", defaultRunOutputLimit(), "number of parallel runoutput tests to run")
 	generics       = flag.String("G", "0,3", "a comma-separated list of -G compiler flags to test with")
+	force          = flag.Bool("f", false, "run expected-failure generics tests rather than skipping them")
 
 	shard  = flag.Int("shard", 0, "shard index to run. Only applicable if -shards is non-zero.")
 	shards = flag.Int("shards", 0, "number of shards. If 0, all tests are run. This is used by the continuous build.")
@@ -518,7 +519,7 @@ func (t *test) run() {
 		close(t.donec)
 	}()
 
-	if t.glevel > 0 {
+	if t.glevel > 0 && !*force {
 		// Files excluded from generics testing.
 		filename := strings.Replace(t.goFileName(), "\\", "/", -1) // goFileName() uses \ on Windows
 		if excludedFiles[filename] {
@@ -657,33 +658,37 @@ func (t *test) run() {
 		Compile
 	)
 
-	// checkFlags reports whether the current test configuration should
-	// be skipped because flags (which should be an arguments list for
-	// "go tool compile", not "go build") contains an excluded flag.
-	// It will also update flags as appropriate.
-	checkFlags := func(tool Tool) bool {
-		if t.glevel > 0 {
+	// validForGLevel reports whether the current test is valid to 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
 			return true
 		}
 
+		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
+			}
+		}
+
 		switch tool {
 		case Build, Run:
 			// ok; handled in goGcflags
 
 		case Compile:
-			for _, flag := range flags {
-				for _, pattern := range excludedFlags {
-					if strings.Contains(flag, pattern) {
-						if *verbose {
-							fmt.Printf("excl\t%s\t%s\n", t.goFileName(), flags)
-						}
-						return true // cannot handle flag
-					}
-				}
-			}
 			flags = append(flags, fmt.Sprintf("-G=%v", t.glevel))
 
 		default:
+			// we don't know how to add -G for this test yet
+			if *verbose {
+				fmt.Printf("excl\t%s\n", t.goFileName())
+			}
 			return false
 		}
 
@@ -766,7 +771,7 @@ func (t *test) run() {
 		t.err = fmt.Errorf("unimplemented action %q", action)
 
 	case "asmcheck":
-		if !checkFlags(AsmCheck) {
+		if !validForGLevel(AsmCheck) {
 			return
 		}
 
@@ -824,7 +829,7 @@ func (t *test) run() {
 		return
 
 	case "errorcheck":
-		if !checkFlags(Compile) {
+		if !validForGLevel(Compile) {
 			return
 		}
 
@@ -858,7 +863,7 @@ func (t *test) run() {
 		t.err = t.errorCheck(string(out), wantAuto, long, t.gofile)
 
 	case "compile":
-		if !checkFlags(Compile) {
+		if !validForGLevel(Compile) {
 			return
 		}
 
@@ -866,7 +871,7 @@ func (t *test) run() {
 		_, t.err = compileFile(runcmd, long, flags)
 
 	case "compiledir":
-		if !checkFlags(Compile) {
+		if !validForGLevel(Compile) {
 			return
 		}
 
@@ -885,7 +890,7 @@ func (t *test) run() {
 		}
 
 	case "errorcheckdir", "errorcheckandrundir":
-		if !checkFlags(Compile) {
+		if !validForGLevel(Compile) {
 			return
 		}
 
@@ -934,7 +939,7 @@ func (t *test) run() {
 		fallthrough
 
 	case "rundir":
-		if !checkFlags(Run) {
+		if !validForGLevel(Run) {
 			return
 		}
 
@@ -996,7 +1001,7 @@ func (t *test) run() {
 		}
 
 	case "runindir":
-		if !checkFlags(Run) {
+		if !validForGLevel(Run) {
 			return
 		}
 
@@ -1039,7 +1044,7 @@ func (t *test) run() {
 		t.checkExpectedOutput(out)
 
 	case "build":
-		if !checkFlags(Build) {
+		if !validForGLevel(Build) {
 			return
 		}
 
@@ -1050,7 +1055,7 @@ func (t *test) run() {
 		}
 
 	case "builddir", "buildrundir":
-		if !checkFlags(Build) {
+		if !validForGLevel(Build) {
 			return
 		}
 
@@ -1133,7 +1138,7 @@ func (t *test) run() {
 		}
 
 	case "buildrun":
-		if !checkFlags(Build) {
+		if !validForGLevel(Build) {
 			return
 		}
 
@@ -1162,7 +1167,7 @@ func (t *test) run() {
 		t.checkExpectedOutput(out)
 
 	case "run":
-		if !checkFlags(Run) {
+		if !validForGLevel(Run) {
 			return
 		}
 
@@ -1209,7 +1214,7 @@ func (t *test) run() {
 		t.checkExpectedOutput(out)
 
 	case "runoutput":
-		if !checkFlags(Run) {
+		if !validForGLevel(Run) {
 			return
 		}
 
@@ -1248,7 +1253,7 @@ func (t *test) run() {
 		t.checkExpectedOutput(out)
 
 	case "errorcheckoutput":
-		if !checkFlags(Compile) {
+		if !validForGLevel(Compile) {
 			return
 		}
 
@@ -2015,15 +2020,6 @@ func overlayDir(dstRoot, srcRoot string) error {
 // checking are also excluded since these phases are not running yet.
 // We can get rid of this code once types2 is fully plugged in.
 
-// For now we skip tests when we can't handle the file or some of the flags.
-// The first goal is to eliminate the excluded list; the second goal is to
-// eliminate the flag list.
-
-var excludedFlags = []string{
-	"-G", // skip redundant testing
-	"-m",
-}
-
 // 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.
 var excludedFiles = map[string]bool{
@@ -2101,4 +2097,39 @@ var excludedFiles = map[string]bool{
 	"fixedbugs/issue7525e.go":  true, // types2 reports init cycle error on different line - ok otherwise
 	"fixedbugs/issue7525.go":   true, // types2 reports init cycle error on different line - ok otherwise
 	"fixedbugs/issue9691.go":   true, // "cannot assign to int(.autotmp_4)" (probably irgen's fault)
+
+	// tests that rely on -m diagnostics, which currently differ with -G=3
+	//
+	// TODO(mdempsky): Triage, though most of the issues seem to fall into:
+	// - Anonymous result parameters given different names (e.g., ~r0 vs ~r1)
+	// - Some escape analysis diagnostics being printed without position information
+	// - Some expressions printed differently (e.g., "int(100)" instead
+	//   of "100" or "&composite literal" instead of "&[4]int{...}").
+	"closure3.go":              true,
+	"escape2.go":               true,
+	"escape2n.go":              true,
+	"escape4.go":               true,
+	"escape5.go":               true,
+	"escape_array.go":          true,
+	"escape_calls.go":          true,
+	"escape_field.go":          true,
+	"escape_iface.go":          true,
+	"escape_indir.go":          true,
+	"escape_level.go":          true,
+	"escape_map.go":            true,
+	"escape_param.go":          true,
+	"escape_slice.go":          true,
+	"escape_struct_param1.go":  true,
+	"escape_struct_param2.go":  true,
+	"fixedbugs/issue12006.go":  true,
+	"fixedbugs/issue13799.go":  true,
+	"fixedbugs/issue21709.go":  true,
+	"fixedbugs/issue24651a.go": true,
+	"fixedbugs/issue24651b.go": true,
+	"fixedbugs/issue27557.go":  true,
+	"fixedbugs/issue31573.go":  true,
+	"fixedbugs/issue37837.go":  true,
+	"fixedbugs/issue39292.go":  true,
+	"fixedbugs/issue7921.go":   true,
+	"inline.go":                true,
 }