[release-branch.go1.12] cmd/cgo: simplify and fix handling of untyped constants

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 #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>
(cherry picked from commit 711ea1e716b0c620cd9bcdd405eccae230d6dcbb)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165748
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/misc/cgo/test/testdata/issue30527.go b/misc/cgo/test/testdata/issue30527.go
new file mode 100644 (file)
index 0000000..4ea7d31
--- /dev/null
+++ 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
new file mode 100644 (file)
index 0000000..eb50147
--- /dev/null
+++ 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
new file mode 100644 (file)
index 0000000..87e8255
--- /dev/null
+++ 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
index 83d727a8a5da6846865076daff262df8ef7b01f9..54d6bc2559dc9d901d7ac8ba6b59a6bb30426351 100644 (file)
--- 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
index b5cf04cf4c7bd3e50daae03ea0ea451b143be955..11a5472786d761b4f4089f711793cf33eca02bfe 100644 (file)
--- 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
index 80435b0634d901e98f104ec7a65975b450008011..11aeee4aab74f661d8f3a62197c6ff5faef64ba4 100644 (file)
--- 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
 }