cmd/cgo: simplify and fix handling of untyped constants

remyoudompheng · ianlancetaylor · commit 711ea1e716b0 · 2019-03-06T21:16:20.000Z
Instead of trying to guess type of constants in the AST, which is hard, use the "var cgo%d Type = Constant" so that typechecking is left to the Go compiler. The previous code could still fail in some cases for constants imported from other modules or defined in other, non-cgo files. Fixes golang#30527 Change-Id: I2120cd90e90a74b9d765eeec53f6a3d2cfc1b642 Reviewed-on: https://go-review.googlesource.com/c/go/+/164897 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/misc/cgo/test/testdata/issue30527.go b/misc/cgo/test/testdata/issue30527.go
@@ -0,0 +1,14 @@
+// Copyright 2019 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.
+
+// Issue 30527: function call rewriting casts untyped
+// constants to int because of ":=" usage.
+
+package cgotest
+
+import "cgotest/issue30527"
+
+func issue30527G() {
+	issue30527.G(nil)
+}
diff --git a/misc/cgo/test/testdata/issue30527/a.go b/misc/cgo/test/testdata/issue30527/a.go
@@ -0,0 +1,19 @@
+// Copyright 2019 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 issue30527
+
+import "math"
+
+/*
+#include <inttypes.h>
+
+static void issue30527F(char **p, uint64_t mod, uint32_t unused) {}
+*/
+import "C"
+
+func G(p **C.char) {
+	C.issue30527F(p, math.MaxUint64, 1)
+	C.issue30527F(p, 1<<64-1, Z)
+}
diff --git a/misc/cgo/test/testdata/issue30527/b.go b/misc/cgo/test/testdata/issue30527/b.go
@@ -0,0 +1,11 @@
+// Copyright 2019 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 issue30527
+
+const (
+	X = 1 << iota
+	Y
+	Z
+)
diff --git a/src/cmd/cgo/ast.go b/src/cmd/cgo/ast.go
@@ -200,18 +200,6 @@ func (f *File) saveExprs(x interface{}, context astContext) {
 		}
 	case *ast.CallExpr:
 		f.saveCall(x, context)
-	case *ast.GenDecl:
-		if x.Tok == token.CONST {
-			for _, spec := range x.Specs {
-				vs := spec.(*ast.ValueSpec)
-				if vs.Type == nil {
-					for _, name := range spec.(*ast.ValueSpec).Names {
-						consts[name.Name] = true
-					}
-				}
-			}
-		}
-
 	}
 }
 
diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
@@ -897,21 +897,16 @@ func (p *Package) rewriteCall(f *File, call *Call) (string, bool) {
 			needsUnsafe = true
 		}
 
-		// Explicitly convert untyped constants to the
-		// parameter type, to avoid a type mismatch.
-		if p.isConst(f, arg) {
-			ptype := p.rewriteUnsafe(param.Go)
+		// Use "var x T = ..." syntax to explicitly convert untyped
+		// constants to the parameter type, to avoid a type mismatch.
+		ptype := p.rewriteUnsafe(param.Go)
+
+		if !p.needsPointerCheck(f, param.Go, args[i]) {
 			if ptype != param.Go {
 				needsUnsafe = true
 			}
-			arg = &ast.CallExpr{
-				Fun:  ptype,
-				Args: []ast.Expr{arg},
-			}
-		}
-
-		if !p.needsPointerCheck(f, param.Go, args[i]) {
-			fmt.Fprintf(&sb, "_cgo%d := %s; ", i, gofmtPos(arg, origArg.Pos()))
+			fmt.Fprintf(&sb, "var _cgo%d %s = %s; ", i,
+				gofmtLine(ptype), gofmtPos(arg, origArg.Pos()))
 			continue
 		}
 
@@ -1254,47 +1249,6 @@ func (p *Package) isType(t ast.Expr) bool {
 	return false
 }
 
-// isConst reports whether x is an untyped constant expression.
-func (p *Package) isConst(f *File, x ast.Expr) bool {
-	switch x := x.(type) {
-	case *ast.BasicLit:
-		return true
-	case *ast.SelectorExpr:
-		id, ok := x.X.(*ast.Ident)
-		if !ok || id.Name != "C" {
-			return false
-		}
-		name := f.Name[x.Sel.Name]
-		if name != nil {
-			return name.IsConst()
-		}
-	case *ast.Ident:
-		return x.Name == "nil" ||
-			strings.HasPrefix(x.Name, "_Ciconst_") ||
-			strings.HasPrefix(x.Name, "_Cfconst_") ||
-			strings.HasPrefix(x.Name, "_Csconst_") ||
-			consts[x.Name]
-	case *ast.UnaryExpr:
-		return p.isConst(f, x.X)
-	case *ast.BinaryExpr:
-		return p.isConst(f, x.X) && p.isConst(f, x.Y)
-	case *ast.ParenExpr:
-		return p.isConst(f, x.X)
-	case *ast.CallExpr:
-		// Calling the builtin function complex on two untyped
-		// constants returns an untyped constant.
-		// TODO: It's possible to construct a case that will
-		// erroneously succeed if there is a local function
-		// named "complex", shadowing the builtin, that returns
-		// a numeric type. I can't think of any cases that will
-		// erroneously fail.
-		if id, ok := x.Fun.(*ast.Ident); ok && id.Name == "complex" && len(x.Args) == 2 {
-			return p.isConst(f, x.Args[0]) && p.isConst(f, x.Args[1])
-		}
-	}
-	return false
-}
-
 // isVariable reports whether x is a variable, possibly with field references.
 func (p *Package) isVariable(x ast.Expr) bool {
 	switch x := x.(type) {
diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
@@ -71,9 +71,6 @@ type File struct {
 	Edit     *edit.Buffer
 }
 
-// Untyped constants in the current package.
-var consts = make(map[string]bool)
-
 func (f *File) offset(p token.Pos) int {
 	return fset.Position(p).Offset
 }

Original file line number	Diff line number	Diff line change
`@@ -200,18 +200,6 @@ func (f *File) saveExprs(x interface{}, context astContext) {`
`200`	`200`	`}`
`201`	`201`	`case *ast.CallExpr:`
`202`	`202`	`f.saveCall(x, context)`
`203`		`- case *ast.GenDecl:`
`204`		`- if x.Tok == token.CONST {`
`205`		`- for _, spec := range x.Specs {`
`206`		`- vs := spec.(*ast.ValueSpec)`
`207`		`- if vs.Type == nil {`
`208`		`- for _, name := range spec.(*ast.ValueSpec).Names {`
`209`		`- consts[name.Name] = true`
`210`		`- }`
`211`		`- }`
`212`		`- }`
`213`		`- }`
`214`		`-`
`215`	`203`	`}`
`216`	`204`	`}`
`217`	`205`
Original file line number	Diff line number	Diff line change
`@@ -71,9 +71,6 @@ type File struct {`
`71`	`71`	`Edit *edit.Buffer`
`72`	`72`	`}`
`73`	`73`
`74`		`-// Untyped constants in the current package.`
`75`		`-var consts = make(map[string]bool)`
`76`		`-`
`77`	`74`	`func (f *File) offset(p token.Pos) int {`
`78`	`75`	`return fset.Position(p).Offset`
`79`	`76`	`}`