From 1e2114349d995ce09c75411463c4cdb59d40d8fc Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 4 Sep 2024 13:20:36 -0700 Subject: [PATCH] cmd/compile/internal/syntax: handle parentheses around constraints consistently 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 #69206. Change-Id: I974a800265625e8daf9477faa9ee4dd74dbd17ce Reviewed-on: https://go-review.googlesource.com/c/go/+/610758 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley Auto-Submit: Robert Griesemer Reviewed-by: Robert Griesemer --- src/cmd/compile/internal/syntax/parser.go | 16 ++++++++++++++-- src/cmd/compile/internal/syntax/printer.go | 12 ++++++++---- src/cmd/compile/internal/syntax/printer_test.go | 6 ++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go index 20106f4e61aee0..cd6b6696a2d0c8 100644 --- 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 index 9f20db54dea5b9..3b234d43f9b8d0 100644 --- 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 index 99baf7f5b63c96..22585dfd259b24 100644 --- 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 }