go/printer: Fprint crashes with unexpected parenthesized expression panic

[mateusz834]

Reproducer:

func TestPrinterPanic(t *testing.T) {
	const src = `package A
type A[C((D)),] struct{}
`

	fs := token.NewFileSet()
	f, err := parser.ParseFile(fs, "test.go", src, parser.ParseComments|parser.SkipObjectResolution)
	if err != nil {
		t.Fatal(err)
	}
	Fprint(io.Discard, fs, f) // panic: unexpected parenthesized expression.
}

panic: unexpected parenthesized expression [recovered]
        panic: unexpected parenthesized expression

goroutine 6 [running]:
testing.tRunner.func1.2({0x586300, 0x5fbc80})
        testing/testing.go:1706 +0x230
testing.tRunner.func1()
        testing/testing.go:1709 +0x35e
panic({0x586300?, 0x5fbc80?})
        runtime/panic.go:785 +0x132
go/printer.combinesWithName({0x5fe0b8?, 0xc0000700e0?})
        go/printer/nodes.go:416 +0x5c
go/printer.(*printer).parameters(0xc0000a21a0, 0xc0000845a0, 0x2)
        go/printer/nodes.go:383 +0xa3d
go/printer.(*printer).spec(0xc0000a21a0, {0x5fe358?, 0xc000090a40}, 0x1, 0x1?)
        go/printer/nodes.go:1719 +0x419
go/printer.(*printer).genDecl(0xc0000a21a0, 0xc000090b00)
        go/printer/nodes.go:1777 +0x407
go/printer.(*printer).decl(0xc0000a21a0?, {0x5fdfc8?, 0xc000090b00?})
        go/printer/nodes.go:1945 +0x51
go/printer.(*printer).declList(0xc0000a21a0, {0xc000020460?, 0xc000070060?, 0x0?})
        go/printer/nodes.go:1990 +0x39
go/printer.(*printer).file(0xc0000a21a0, 0xc000080280)
        go/printer/nodes.go:1999 +0x16f
go/printer.(*printer).printNode(0xc0000a21a0, {0x592020?, 0xc000080280?})
        go/printer/printer.go:1172 +0x16a
go/printer.(*Config).fprint(0xc0000c9f20, {0x5fcea0, 0x72a020}, 0x586300?, {0x592020, 0xc000080280}, 0x4dabc0?)
        go/printer/printer.go:1364 +0xab
go/printer.(*Config).Fprint(...)
        go/printer/printer.go:1424
go/printer.Fprint(...)
        go/printer/printer.go:1432

[gabyhelp]

Related Issues and Documentation

• go/format: any input to format.Source panics #42300 (closed)
• cmd/compile: compiler crash on failing to get Right parenthesis in Go1.9 #19667 (closed)
• go/printer, cmd/gofmt: panic on large Go file of length >= 1<<30 #30121
• cmd/compile: panic while printing error when cross-compiling #20789 (closed)
• cmd/compile: generic types implementing fmt.Stringer crash fmt.Printf #47740 (closed)
• fmt: Print panics when printing a recursive object #44942 (closed)
• gopls panics on interface conversion in printf/printf.go #33911 (closed)
• runtime: fmt.Sprintf crash the process #56725 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mateusz834]

I wonder whether this is the right fix:

diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index a4651e0608..495ec22031 100644
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -411,9 +411,7 @@ func combinesWithName(x ast.Expr) bool {
        case *ast.BinaryExpr:
                return combinesWithName(x.X) && !isTypeElem(x.Y)
        case *ast.ParenExpr:
-               // name(x) combines but we are making sure at
-               // the call site that x is never parenthesized.
-               panic("unexpected parenthesized expression")
+               return combinesWithName(x.X)
        }
        return false
 }

[mateusz834]

But also looking at the parser, I wonder whether the parser is right here:

go/src/go/parser/parser.go

Lines 2695 to 2702 in fc9f02c

	case *ast.CallExpr:
	if name, _ := x.Fun.(*ast.Ident); name != nil {
	if len(x.Args) == 1 && x.Ellipsis == token.NoPos && (force || isTypeElem(x.Args[0])) {
	// x = name "(" x.ArgList[0] ")"
	return name, x.Args[0]
	}
	}
	}

We are loosing some bits of the AST, i believe it should be converted to *ast.ParenExpr:

diff --git a/src/go/parser/parser.go b/src/go/parser/parser.go
index 17808b366f..6c418fd796 100644
--- a/src/go/parser/parser.go
+++ b/src/go/parser/parser.go
@@ -2695,8 +2695,11 @@ func extractName(x ast.Expr, force bool) (*ast.Ident, ast.Expr) {
        case *ast.CallExpr:
                if name, _ := x.Fun.(*ast.Ident); name != nil {
                        if len(x.Args) == 1 && x.Ellipsis == token.NoPos && (force || isTypeElem(x.Args[0])) {
-                               // x = name "(" x.ArgList[0] ")"
-                               return name, x.Args[0]
+                               return name, &ast.ParenExpr{
+                                       Lparen: x.Lparen,
+                                       X:      x.Args[0],
+                                       Rparen: x.Rparen,
+                               }
                        }
                }
        }

[gopherbot]

Change https://go.dev/cl/610115 mentions this issue: go/printer: do not panic on *ast.ParenExpr in combinesWithName

[ianlancetaylor]

CC @griesemer

[griesemer]

I think this looks right. Will comment on the CL.

[gopherbot]

Change https://go.dev/cl/610758 mentions this issue: cmd/compile/internal/syntax: handle parentheses around constraints consistently

[gopherbot]

Change https://go.dev/cl/611355 mentions this issue: go/printer: check whether !isTypeElem, instead of combinesWithName when ParenExpr