cmd/compile/internal/syntax: handle parentheses around constraints co…

…nsistently Generally, the parser strips (i.e., does not record in the syntax tree) unnecessary parentheses. Specifically, given a type parameter list of the form [P (C),] it records it as [P C] and then no comma is required when printing. However it did only strip one level of parentheses, and [P ((C)),] made it through, causing a panic when printing. Somewhat related, the printer stripped parentheses around constraints as well. This CL implements a more consistent behavior: 1) The parser strips all parentheses around constraints. For testing purposes, a local flag (keep_parens) can be set to retain the parentheses. 2) The printer code now correctly intruces a comma if parentheses are present (e.g., when testing with keep_parens). This case does not occur in normal operation. 3) The printer does not strip parentheses around constraints since the parser does it already. For golang#69206. Change-Id: I974a800265625e8daf9477faa9ee4dd74dbd17ce Reviewed-on: https://go-review.googlesource.com/c/go/+/610758 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
aytanet · Sep 5, 2024 · 1e21143 · 1e21143
1 parent 32bd777
commit 1e21143
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go
@@ -721,8 +721,20 @@ func extractName(x Expr, force bool) (*Name, Expr) {
 	case *CallExpr:
 		if name, _ := x.Fun.(*Name); name != nil {
 			if len(x.ArgList) == 1 && !x.HasDots && (force || isTypeElem(x.ArgList[0])) {
-				// x = name "(" x.ArgList[0] ")"
-				return name, x.ArgList[0]
+				// The parser doesn't keep unnecessary parentheses.
+				// Set the flag below to keep them, for testing
+				// (see go.dev/issues/69206).
+				const keep_parens = false
+				if keep_parens {
+					// x = name (x.ArgList[0])
+					px := new(ParenExpr)
+					px.pos = x.pos // position of "(" in call
+					px.X = x.ArgList[0]
+					return name, px
+				} else {
+					// x = name x.ArgList[0]
+					return name, Unparen(x.ArgList[0])
+				}
 			}
 		}
 	}

diff --git a/src/cmd/compile/internal/syntax/printer.go b/src/cmd/compile/internal/syntax/printer.go
@@ -916,7 +916,7 @@ func (p *printer) printParameterList(list []*Field, tok token) {
 			}
 			p.print(blank)
 		}
-		p.printNode(Unparen(f.Type)) // no need for (extra) parentheses around parameter types
+		p.printNode(f.Type)
 	}
 	// A type parameter list [P T] where the name P and the type expression T syntactically
 	// combine to another valid (value) expression requires a trailing comma, as in [P *T,]
@@ -943,9 +943,13 @@ func combinesWithName(x Expr) bool {
 		// binary expressions
 		return combinesWithName(x.X) && !isTypeElem(x.Y)
 	case *ParenExpr:
-		// name(x) combines but we are making sure at
-		// the call site that x is never parenthesized.
-		panic("unexpected parenthesized expression")
+		// Note that the parser strips parentheses in these cases
+		// (see extractName, parser.typeOrNil) unless keep_parens
+		// is set, so we should never reach here.
+		// Do the right thing (rather than panic) for testing and
+		// in case we change parser behavior.
+		// See also go.dev/issues/69206.
+		return !isTypeElem(x.X)
 	}
 	return false
 }

diff --git a/src/cmd/compile/internal/syntax/printer_test.go b/src/cmd/compile/internal/syntax/printer_test.go
@@ -134,6 +134,12 @@ var stringTests = [][2]string{
 	dup("package p; type _ chan<- <-chan int"),
 	dup("package p; type _ chan<- chan<- int"),
 
+	// go.dev/issues/69206
+	dup("package p; type _[P C] int"),
+	{"package p; type _[P (C),] int", "package p; type _[P C] int"},
+	{"package p; type _[P ((C)),] int", "package p; type _[P C] int"},
+	{"package p; type _[P, Q ((C))] int", "package p; type _[P, Q C] int"},
+
 	// TODO(gri) expand
 }