From 6db8a00c756207a4a6f272911d1512b838674b1a Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 16 Jun 2014 12:29:30 -0400 Subject: [PATCH] go.tools/go/ssa: write zero value when storing a composite literal in-place if necessary Previously, statements such as: type T struct { a, b int } [...] x = T{} x = T{b: 1} would only affect the aggregate members mentioned in the composite literal and leave the other members unchanged. This change causes us to write a zero value to the target in cases where the target is not already known to hold a zero value and the number of initializers in the composite literal differs from the number of elements in its type. Author: Peter Collingbourne. (hg clpatch got confused) LGTM=pcc R=pcc CC=golang-codereviews https://golang.org/cl/107980045 --- go/ssa/builder.go | 36 +++++++++++++------- go/ssa/emit.go | 27 ++++++++++----- go/ssa/interp/testdata/coverage.go | 53 ++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 20 deletions(-) diff --git a/go/ssa/builder.go b/go/ssa/builder.go index 3249a728e1..532a7993c2 100644 --- a/go/ssa/builder.go +++ b/go/ssa/builder.go @@ -344,7 +344,7 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue { v = fn.addLocal(t, e.Lbrace) } v.Comment = "complit" - b.compLit(fn, v, e) // initialize in place + b.compLit(fn, v, e, true) // initialize in place return &address{addr: v, expr: e} case *ast.ParenExpr: @@ -406,13 +406,14 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue { } // exprInPlace emits to fn code to initialize the lvalue loc with the -// value of expression e. +// value of expression e. If isZero is true, exprInPlace assumes that loc +// holds the zero value for its type. // // This is equivalent to loc.store(fn, b.expr(fn, e)) but may // generate better code in some cases, e.g. for composite literals // in an addressable location. // -func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr) { +func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr, isZero bool) { if e, ok := unparen(e).(*ast.CompositeLit); ok { // A CompositeLit never evaluates to a pointer, // so if the type of the location is a pointer, @@ -432,7 +433,7 @@ func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr) { // Fall back to copying. } else { addr := loc.address(fn) - b.compLit(fn, addr, e) // in place + b.compLit(fn, addr, e, isZero) // in place emitDebugRef(fn, e, addr, true) return } @@ -910,7 +911,7 @@ func (b *builder) localValueSpec(fn *Function, spec *ast.ValueSpec) { fn.addLocalForIdent(id) } lval := b.addr(fn, id, false) // non-escaping - b.exprInPlace(fn, lval, spec.Values[i]) + b.exprInPlace(fn, lval, spec.Values[i], true) } case len(spec.Values) == 0: @@ -967,7 +968,7 @@ func (b *builder) assignStmt(fn *Function, lhss, rhss []ast.Expr, isDef bool) { // x = type{...} // Optimization: in-place construction // of composite literals. - b.exprInPlace(fn, lvals[0], rhss[0]) + b.exprInPlace(fn, lvals[0], rhss[0], false) } else { // Parallel assignment. All reads must occur // before all updates, precluding exprInPlace. @@ -1010,17 +1011,22 @@ func (b *builder) arrayLen(fn *Function, elts []ast.Expr) int64 { // compLit emits to fn code to initialize a composite literal e at // address addr with type typ, typically allocated by Alloc. // Nested composite literals are recursively initialized in place -// where possible. +// where possible. If isZero is true, compLit assumes that addr +// holds the zero value for typ. // // A CompositeLit may have pointer type only in the recursive (nested) // case when the type name is implicit. e.g. in []*T{{}}, the inner // literal has type *T behaves like &T{}. // In that case, addr must hold a T, not a *T. // -func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit) { +func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero bool) { typ := deref(fn.Pkg.typeOf(e)) switch t := typ.Underlying().(type) { case *types.Struct: + if !isZero && len(e.Elts) != t.NumFields() { + emitMemClear(fn, addr) + isZero = true + } for i, e := range e.Elts { fieldIndex := i if kv, ok := e.(*ast.KeyValueExpr); ok { @@ -1041,7 +1047,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit) { } faddr.setType(types.NewPointer(sf.Type())) fn.emit(faddr) - b.exprInPlace(fn, &address{addr: faddr, expr: e}, e) + b.exprInPlace(fn, &address{addr: faddr, expr: e}, e, isZero) } case *types.Array, *types.Slice: @@ -1053,11 +1059,17 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit) { alloc := emitNew(fn, at, e.Lbrace) alloc.Comment = "slicelit" array = alloc + isZero = true case *types.Array: at = t array = addr } + if !isZero && int64(len(e.Elts)) != at.Len() { + emitMemClear(fn, array) + isZero = true + } + var idx *Const for _, e := range e.Elts { if kv, ok := e.(*ast.KeyValueExpr); ok { @@ -1076,7 +1088,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit) { } iaddr.setType(types.NewPointer(at.Elem())) fn.emit(iaddr) - b.exprInPlace(fn, &address{addr: iaddr, expr: e}, e) + b.exprInPlace(fn, &address{addr: iaddr, expr: e}, e, isZero) } if t != at { // slice s := &Slice{X: array} @@ -1098,7 +1110,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit) { t: t.Elem(), pos: e.Colon, } - b.exprInPlace(fn, loc, e.Value) + b.exprInPlace(fn, loc, e.Value, true) } default: @@ -2196,7 +2208,7 @@ func (p *Package) Build() { } else { lval = blank{} } - b.exprInPlace(init, lval, varinit.Rhs) + b.exprInPlace(init, lval, varinit.Rhs, true) } else { // n:1 initialization: var x, y := f() tuple := b.exprN(init, varinit.Rhs) diff --git a/go/ssa/emit.go b/go/ssa/emit.go index 5cb110f237..c5d23fda6b 100644 --- a/go/ssa/emit.go +++ b/go/ssa/emit.go @@ -415,6 +415,24 @@ func emitFieldSelection(f *Function, v Value, index int, wantAddr bool, pos toke return v } +// zeroValue emits to f code to produce a zero value of type t, +// and returns it. +// +func zeroValue(f *Function, t types.Type) Value { + switch t.Underlying().(type) { + case *types.Struct, *types.Array: + return emitLoad(f, f.addLocal(t, token.NoPos)) + default: + return zeroConst(t) + } +} + +// emitMemClear emits to f code to zero the value pointed to by ptr. +func emitMemClear(f *Function, ptr Value) { + // TODO(adonovan): define and use a 'memclr' intrinsic for aggregate types. + emitStore(f, ptr, zeroValue(f, deref(ptr.Type()))) +} + // createRecoverBlock emits to f a block of code to return after a // recovered panic, and sets f.Recover to it. // @@ -443,16 +461,9 @@ func createRecoverBlock(f *Function) { R := f.Signature.Results() for i, n := 0, R.Len(); i < n; i++ { T := R.At(i).Type() - var v Value // Return zero value of each result type. - switch T.Underlying().(type) { - case *types.Struct, *types.Array: - v = emitLoad(f, f.addLocal(T, token.NoPos)) - default: - v = zeroConst(T) - } - results = append(results, v) + results = append(results, zeroValue(f, T)) } } f.emit(&Return{Results: results}) diff --git a/go/ssa/interp/testdata/coverage.go b/go/ssa/interp/testdata/coverage.go index 3ad49f2084..dff77c4660 100644 --- a/go/ssa/interp/testdata/coverage.go +++ b/go/ssa/interp/testdata/coverage.go @@ -690,3 +690,56 @@ func init() { i.f() panic("unreachable") } + +// Test for in-place initialization. +func init() { + // struct + type S struct { + a, b int + } + s := S{1, 2} + s = S{b: 3} + if s.a != 0 { + panic("s.a != 0") + } + if s.b != 3 { + panic("s.b != 3") + } + s = S{} + if s.a != 0 { + panic("s.a != 0") + } + if s.b != 0 { + panic("s.b != 0") + } + + // array + type A [4]int + a := A{2, 4, 6, 8} + a = A{1: 6, 2: 4} + if a[0] != 0 { + panic("a[0] != 0") + } + if a[1] != 6 { + panic("a[1] != 6") + } + if a[2] != 4 { + panic("a[2] != 4") + } + if a[3] != 0 { + panic("a[3] != 0") + } + a = A{} + if a[0] != 0 { + panic("a[0] != 0") + } + if a[1] != 0 { + panic("a[1] != 0") + } + if a[2] != 0 { + panic("a[2] != 0") + } + if a[3] != 0 { + panic("a[3] != 0") + } +}