diff options
author | Keith Randall <khr@golang.org> | 2017-09-26 15:14:50 -0700 |
---|---|---|
committer | Keith Randall <khr@golang.org> | 2017-11-17 22:11:03 +0000 |
commit | b868616b63a82a4f5917400b2df63a19ebe041e2 (patch) | |
tree | e07f6aca3643163ea9bb74053797a18f9f8e2bfe /src/cmd/fix | |
parent | 644787c337f7fde20a0676f843bb12378c8f885e (diff) | |
download | go-git-b868616b63a82a4f5917400b2df63a19ebe041e2.tar.gz |
cmd/cgo: special case C ptr types to use uintptr
Some C types are declared as pointers, but C code
stores non-pointers in them. When the Go garbage
collector sees such a pointer, it gets unhappy.
Instead, for these types represent them on the Go
side with uintptr.
We need this change to handle Apple's CoreFoundation
CF*Ref types. Users of these types might need to
update their code like we do in root_cgo_darwin.go.
The only change that is required under normal
circumstances is converting some nils to 0.
A go fix module is provided to help.
Fixes #21897
RELNOTE=yes
Change-Id: I9716cfb255dc918792625f42952aa171cd31ec1b
Reviewed-on: https://go-review.googlesource.com/66332
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Diffstat (limited to 'src/cmd/fix')
-rw-r--r-- | src/cmd/fix/cftype.go | 93 | ||||
-rw-r--r-- | src/cmd/fix/cftype_test.go | 185 | ||||
-rw-r--r-- | src/cmd/fix/typecheck.go | 56 |
3 files changed, 334 insertions, 0 deletions
diff --git a/src/cmd/fix/cftype.go b/src/cmd/fix/cftype.go new file mode 100644 index 0000000000..da1627fbfb --- /dev/null +++ b/src/cmd/fix/cftype.go @@ -0,0 +1,93 @@ +// Copyright 2017 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 main + +import ( + "go/ast" + "go/token" + "reflect" + "strings" +) + +func init() { + register(cftypeFix) +} + +var cftypeFix = fix{ + name: "cftype", + date: "2017-09-27", + f: cftypefix, + desc: `Fixes initializers of C.CF*Ptr types`, + disabled: false, +} + +// Old state: +// type CFTypeRef unsafe.Pointer +// New state: +// type CFTypeRef uintptr +// and similar for other CF*Ref types. +// This fix finds nils initializing these types and replaces the nils with 0s. +func cftypefix(f *ast.File) bool { + if !imports(f, "C") { + return false + } + typeof, _ := typecheck(&TypeConfig{}, f) + + // step 1: Find all the nils with the offending types. + // Compute their replacement. + badNils := map[interface{}]ast.Expr{} + walk(f, func(n interface{}) { + if i, ok := n.(*ast.Ident); ok && i.Name == "nil" && badPointerType(typeof[n]) { + badNils[n] = &ast.BasicLit{ValuePos: i.NamePos, Kind: token.INT, Value: "0"} + } + }) + if len(badNils) == 0 { + return false + } + + // step 2: find all uses of the bad nils, replace them with 0. + // There's no easy way to map from an ast.Expr to all the places that use them, so + // we use reflect to find all such references. + exprType := reflect.TypeOf((*ast.Expr)(nil)).Elem() + exprSliceType := reflect.TypeOf(([]ast.Expr)(nil)) + walk(f, func(n interface{}) { + if n == nil { + return + } + v := reflect.ValueOf(n) + if v.Type().Kind() != reflect.Ptr { + return + } + if v.IsNil() { + return + } + v = v.Elem() + if v.Type().Kind() != reflect.Struct { + return + } + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + if f.Type() == exprType { + if r := badNils[f.Interface()]; r != nil { + f.Set(reflect.ValueOf(r)) + } + } + if f.Type() == exprSliceType { + for j := 0; j < f.Len(); j++ { + e := f.Index(j) + if r := badNils[e.Interface()]; r != nil { + e.Set(reflect.ValueOf(r)) + } + } + } + } + }) + + return true +} + +func badPointerType(s string) bool { + return strings.HasPrefix(s, "C.CF") && strings.HasSuffix(s, "Ref") +} diff --git a/src/cmd/fix/cftype_test.go b/src/cmd/fix/cftype_test.go new file mode 100644 index 0000000000..adaed2114f --- /dev/null +++ b/src/cmd/fix/cftype_test.go @@ -0,0 +1,185 @@ +// Copyright 2017 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 main + +func init() { + addTestCases(cftypeTests, cftypefix) +} + +var cftypeTests = []testCase{ + { + Name: "cftype.localVariable", + In: `package main + +import "C" + +func f() { + var x C.CFTypeRef = nil + x = nil + x, x = nil, nil +} +`, + Out: `package main + +import "C" + +func f() { + var x C.CFTypeRef = 0 + x = 0 + x, x = 0, 0 +} +`, + }, + { + Name: "cftype.globalVariable", + In: `package main + +import "C" + +var x C.CFTypeRef = nil + +func f() { + x = nil +} +`, + Out: `package main + +import "C" + +var x C.CFTypeRef = 0 + +func f() { + x = 0 +} +`, + }, + { + Name: "cftype.EqualArgument", + In: `package main + +import "C" + +var x C.CFTypeRef +var y = x == nil +var z = x != nil +`, + Out: `package main + +import "C" + +var x C.CFTypeRef +var y = x == 0 +var z = x != 0 +`, + }, + { + Name: "cftype.StructField", + In: `package main + +import "C" + +type T struct { + x C.CFTypeRef +} + +var t = T{x: nil} +`, + Out: `package main + +import "C" + +type T struct { + x C.CFTypeRef +} + +var t = T{x: 0} +`, + }, + { + Name: "cftype.FunctionArgument", + In: `package main + +import "C" + +func f(x C.CFTypeRef) { +} + +func g() { + f(nil) +} +`, + Out: `package main + +import "C" + +func f(x C.CFTypeRef) { +} + +func g() { + f(0) +} +`, + }, + { + Name: "cftype.ArrayElement", + In: `package main + +import "C" + +var x = [3]C.CFTypeRef{nil, nil, nil} +`, + Out: `package main + +import "C" + +var x = [3]C.CFTypeRef{0, 0, 0} +`, + }, + { + Name: "cftype.SliceElement", + In: `package main + +import "C" + +var x = []C.CFTypeRef{nil, nil, nil} +`, + Out: `package main + +import "C" + +var x = []C.CFTypeRef{0, 0, 0} +`, + }, + { + Name: "cftype.MapKey", + In: `package main + +import "C" + +var x = map[C.CFTypeRef]int{nil: 0} +`, + Out: `package main + +import "C" + +var x = map[C.CFTypeRef]int{0: 0} +`, + }, + { + Name: "cftype.MapValue", + In: `package main + +import "C" + +var x = map[int]C.CFTypeRef{0: nil} +`, + Out: `package main + +import "C" + +var x = map[int]C.CFTypeRef{0: 0} +`, + }, +} diff --git a/src/cmd/fix/typecheck.go b/src/cmd/fix/typecheck.go index 0352c49db0..58d915869d 100644 --- a/src/cmd/fix/typecheck.go +++ b/src/cmd/fix/typecheck.go @@ -498,6 +498,50 @@ func typecheck1(cfg *TypeConfig, f interface{}, typeof map[interface{}]string, a // T{...} has type T. typeof[n] = gofmt(n.Type) + // Propagate types down to values used in the composite literal. + t := expand(typeof[n]) + if strings.HasPrefix(t, "[") { // array or slice + // Lazy: assume there are no nested [] in the array length. + if i := strings.Index(t, "]"); i >= 0 { + et := t[i+1:] + for _, e := range n.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + e = kv.Value + } + if typeof[e] == "" { + typeof[e] = et + } + } + } + } + if strings.HasPrefix(t, "map[") { // map + // Lazy: assume there are no nested [] in the map key type. + if i := strings.Index(t, "]"); i >= 0 { + kt, vt := t[4:i], t[i+1:] + for _, e := range n.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + if typeof[kv.Key] == "" { + typeof[kv.Key] = kt + } + if typeof[kv.Value] == "" { + typeof[kv.Value] = vt + } + } + } + } + } + if typ := cfg.Type[t]; typ != nil && len(typ.Field) > 0 { // struct + for _, e := range n.Elts { + if kv, ok := e.(*ast.KeyValueExpr); ok { + if ft := typ.Field[fmt.Sprintf("%s", kv.Key)]; ft != "" { + if typeof[kv.Value] == "" { + typeof[kv.Value] = ft + } + } + } + } + } + case *ast.ParenExpr: // (x) has type of x. typeof[n] = typeof[n.X] @@ -579,6 +623,18 @@ func typecheck1(cfg *TypeConfig, f interface{}, typeof map[interface{}]string, a set(res[i], t[i], false) } } + + case *ast.BinaryExpr: + // Propagate types across binary ops that require two args of the same type. + switch n.Op { + case token.EQL, token.NEQ: // TODO: more cases. This is enough for the cftype fix. + if typeof[n.X] != "" && typeof[n.Y] == "" { + typeof[n.Y] = typeof[n.X] + } + if typeof[n.X] == "" && typeof[n.Y] != "" { + typeof[n.X] = typeof[n.Y] + } + } } } walkBeforeAfter(f, before, after) |