From: Robert Griesemer Date: Mon, 9 May 2016 03:59:53 +0000 (-0700) Subject: cmd/compile: fix binary export of composite literals with implicit types X-Git-Tag: go1.7beta1~278 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=87a2ae1fa25677dc9097a25292c54b7b9dac2c9d;p=gostls13.git cmd/compile: fix binary export of composite literals with implicit types Also: - replaced remaining panics with Fatal calls - more comments Fixes #15572. Change-Id: Ifb27e80b66700f5692a84078764a1e928d4b310d Reviewed-on: https://go-review.googlesource.com/22935 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/gc/bexport.go b/src/cmd/compile/internal/gc/bexport.go index aa1915bb6f..cd2963e8e6 100644 --- a/src/cmd/compile/internal/gc/bexport.go +++ b/src/cmd/compile/internal/gc/bexport.go @@ -937,7 +937,7 @@ func parName(f *Field, numbered bool) string { // print symbol with Vargen number or not as desired name := s.Name if strings.Contains(name, ".") { - panic("invalid symbol name: " + name) + Fatalf("invalid symbol name: %s", name) } // Functions that can be inlined use numbered parameters so we can distingish them @@ -1040,6 +1040,8 @@ func (p *exporter) float(x *Mpflt) { // but instead of emitting the information textually, emit the node tree in // binary form. +// TODO(gri) Improve tracing output. The current format is difficult to read. + // stmtList may emit more (or fewer) than len(list) nodes. func (p *exporter) stmtList(list Nodes) { if p.trace { @@ -1116,6 +1118,16 @@ func (p *exporter) expr(n *Node) { defer p.tracef(") ") } + // from nodefmt (fmt.go) + // + // nodefmt reverts nodes back to their original - we don't need to do + // it because we are not bound to produce valid Go syntax when exporting + // + // if (fmtmode != FExp || n.Op != OLITERAL) && n.Orig != nil { + // n = n.Orig + // } + + // from exprfmt (fmt.go) for n != nil && n.Implicit && (n.Op == OIND || n.Op == OADDR) { n = n.Left } @@ -1188,14 +1200,14 @@ func (p *exporter) expr(n *Node) { p.typ(n.Type) } - case OTARRAY, OTMAP, OTCHAN, OTSTRUCT, OTINTER, OTFUNC: - panic("unreachable") // should have been resolved by typechecking + // case OTARRAY, OTMAP, OTCHAN, OTSTRUCT, OTINTER, OTFUNC: + // should have been resolved by typechecking - handled by default case // case OCLOSURE: // unimplemented - handled by default case // case OCOMPLIT: - // unimplemented - handled by default case + // should have been resolved by typechecking - handled by default case case OPTRLIT: p.op(OPTRLIT) @@ -1204,16 +1216,12 @@ func (p *exporter) expr(n *Node) { case OSTRUCTLIT: p.op(OSTRUCTLIT) - if !p.bool(n.Implicit) { - p.typ(n.Type) - } + p.typ(n.Type) p.elemList(n.List) // special handling of field names case OARRAYLIT, OMAPLIT: - p.op(op) - if !p.bool(n.Implicit) { - p.typ(n.Type) - } + p.op(OCOMPLIT) + p.typ(n.Type) p.exprList(n.List) case OKEY: @@ -1226,9 +1234,6 @@ func (p *exporter) expr(n *Node) { case OXDOT, ODOT, ODOTPTR, ODOTINTER, ODOTMETH: p.op(OXDOT) p.expr(n.Left) - if n.Sym == nil { - panic("unreachable") // can this happen during export? - } p.fieldSym(n.Sym, true) case ODOTTYPE, ODOTTYPE2: @@ -1334,7 +1339,8 @@ func (p *exporter) expr(n *Node) { p.op(ODCLCONST) default: - Fatalf("exporter: CANNOT EXPORT: %s\nPlease notify gri@\n", n.Op) + Fatalf("cannot export %s (%d) node\n"+ + "==> please file an issue and assign to gri@\n", n.Op, n.Op) } } @@ -1410,9 +1416,8 @@ func (p *exporter) stmt(n *Node) { p.op(ORETURN) p.exprList(n.List) - case ORETJMP: - // generated by compiler for trampolin routines - not exported - panic("unreachable") + // case ORETJMP: + // unreachable - generated by compiler for trampolin routines case OPROC, ODEFER: p.op(op) @@ -1457,7 +1462,7 @@ func (p *exporter) stmt(n *Node) { p.exprsOrNil(n.Left, nil) case OEMPTY: - // nothing to emit + // nothing to emit case OLABEL: p.op(OLABEL) diff --git a/src/cmd/compile/internal/gc/bimport.go b/src/cmd/compile/internal/gc/bimport.go index c4e6e5dd57..cb375a0ac3 100644 --- a/src/cmd/compile/internal/gc/bimport.go +++ b/src/cmd/compile/internal/gc/bimport.go @@ -233,7 +233,7 @@ func (p *importer) pkg() *Pkg { // an empty path denotes the package we are currently importing; // it must be the first package we see if (path == "") != (len(p.pkgList) == 0) { - panic(fmt.Sprintf("package path %q for pkg index %d", path, len(p.pkgList))) + Fatalf("importer: package path %q for pkg index %d", path, len(p.pkgList)) } pkg := importpkg @@ -821,12 +821,9 @@ func (p *importer) node() *Node { // case OCLOSURE: // unimplemented - // case OCOMPLIT: - // unimplemented - case OPTRLIT: n := p.expr() - if !p.bool() /* !implicit, i.e. '&' operator*/ { + if !p.bool() /* !implicit, i.e. '&' operator */ { if n.Op == OCOMPLIT { // Special case for &T{...}: turn into (*T){...}. n.Right = Nod(OIND, n.Right, nil) @@ -838,18 +835,15 @@ func (p *importer) node() *Node { return n case OSTRUCTLIT: - n := Nod(OCOMPLIT, nil, nil) - if !p.bool() { - n.Right = typenod(p.typ()) - } - n.List.Set(p.elemList()) + n := Nod(OCOMPLIT, nil, typenod(p.typ())) + n.List.Set(p.elemList()) // special handling of field names return n - case OARRAYLIT, OMAPLIT: - n := Nod(OCOMPLIT, nil, nil) - if !p.bool() { - n.Right = typenod(p.typ()) - } + // case OARRAYLIT, OMAPLIT: + // unreachable - mapped to case OCOMPLIT below by exporter + + case OCOMPLIT: + n := Nod(OCOMPLIT, nil, typenod(p.typ())) n.List.Set(p.exprList()) return n @@ -1090,7 +1084,8 @@ func (p *importer) node() *Node { return nil default: - Fatalf("importer: %s (%d) node not yet supported", op, op) + Fatalf("cannot import %s (%d) node\n"+ + "==> please file an issue and assign to gri@\n", op, op) panic("unreachable") // satisfy compiler } } diff --git a/test/fixedbugs/issue15572.dir/a.go b/test/fixedbugs/issue15572.dir/a.go new file mode 100644 index 0000000000..1356601430 --- /dev/null +++ b/test/fixedbugs/issue15572.dir/a.go @@ -0,0 +1,40 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +type T struct { +} + +func F() []T { + return []T{T{}} +} + +func Fi() []T { + return []T{{}} // element with implicit composite literal type +} + +func Fp() []*T { + return []*T{&T{}} +} + +func Fip() []*T { + return []*T{{}} // element with implicit composite literal type +} + +func Gp() map[int]*T { + return map[int]*T{0: &T{}} +} + +func Gip() map[int]*T { + return map[int]*T{0: {}} // element with implicit composite literal type +} + +func Hp() map[*T]int { + return map[*T]int{&T{}: 0} +} + +func Hip() map[*T]int { + return map[*T]int{{}: 0} // key with implicit composite literal type +} diff --git a/test/fixedbugs/issue15572.dir/b.go b/test/fixedbugs/issue15572.dir/b.go new file mode 100644 index 0000000000..355accc880 --- /dev/null +++ b/test/fixedbugs/issue15572.dir/b.go @@ -0,0 +1,27 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package b + +import "./a" + +func F() { + a.F() + a.Fi() +} + +func Fp() { + a.Fp() + a.Fip() +} + +func Gp() { + a.Gp() + a.Gip() +} + +func Hp() { + a.Hp() + a.Hip() +} diff --git a/test/fixedbugs/issue15572.go b/test/fixedbugs/issue15572.go new file mode 100644 index 0000000000..cf77778f66 --- /dev/null +++ b/test/fixedbugs/issue15572.go @@ -0,0 +1,11 @@ +// compiledir + +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Test that exporting composite literals with implicit +// types doesn't crash the typechecker when running over +// inlined function bodies containing such literals. + +package ignored