Skip to content

Commit 711ea1e

Browse files
remyoudomphengianlancetaylor
authored andcommitted
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent edf8539 commit 711ea1e

File tree

6 files changed

+51
-68
lines changed

6 files changed

+51
-68
lines changed

misc/cgo/test/testdata/issue30527.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Issue 30527: function call rewriting casts untyped
6+
// constants to int because of ":=" usage.
7+
8+
package cgotest
9+
10+
import "cgotest/issue30527"
11+
12+
func issue30527G() {
13+
issue30527.G(nil)
14+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package issue30527
6+
7+
import "math"
8+
9+
/*
10+
#include <inttypes.h>
11+
12+
static void issue30527F(char **p, uint64_t mod, uint32_t unused) {}
13+
*/
14+
import "C"
15+
16+
func G(p **C.char) {
17+
C.issue30527F(p, math.MaxUint64, 1)
18+
C.issue30527F(p, 1<<64-1, Z)
19+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package issue30527
6+
7+
const (
8+
X = 1 << iota
9+
Y
10+
Z
11+
)

src/cmd/cgo/ast.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -200,18 +200,6 @@ func (f *File) saveExprs(x interface{}, context astContext) {
200200
}
201201
case *ast.CallExpr:
202202
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-
215203
}
216204
}
217205

src/cmd/cgo/gcc.go

Lines changed: 7 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -897,21 +897,16 @@ func (p *Package) rewriteCall(f *File, call *Call) (string, bool) {
897897
needsUnsafe = true
898898
}
899899

900-
// Explicitly convert untyped constants to the
901-
// parameter type, to avoid a type mismatch.
902-
if p.isConst(f, arg) {
903-
ptype := p.rewriteUnsafe(param.Go)
900+
// Use "var x T = ..." syntax to explicitly convert untyped
901+
// constants to the parameter type, to avoid a type mismatch.
902+
ptype := p.rewriteUnsafe(param.Go)
903+
904+
if !p.needsPointerCheck(f, param.Go, args[i]) {
904905
if ptype != param.Go {
905906
needsUnsafe = true
906907
}
907-
arg = &ast.CallExpr{
908-
Fun: ptype,
909-
Args: []ast.Expr{arg},
910-
}
911-
}
912-
913-
if !p.needsPointerCheck(f, param.Go, args[i]) {
914-
fmt.Fprintf(&sb, "_cgo%d := %s; ", i, gofmtPos(arg, origArg.Pos()))
908+
fmt.Fprintf(&sb, "var _cgo%d %s = %s; ", i,
909+
gofmtLine(ptype), gofmtPos(arg, origArg.Pos()))
915910
continue
916911
}
917912

@@ -1254,47 +1249,6 @@ func (p *Package) isType(t ast.Expr) bool {
12541249
return false
12551250
}
12561251

1257-
// isConst reports whether x is an untyped constant expression.
1258-
func (p *Package) isConst(f *File, x ast.Expr) bool {
1259-
switch x := x.(type) {
1260-
case *ast.BasicLit:
1261-
return true
1262-
case *ast.SelectorExpr:
1263-
id, ok := x.X.(*ast.Ident)
1264-
if !ok || id.Name != "C" {
1265-
return false
1266-
}
1267-
name := f.Name[x.Sel.Name]
1268-
if name != nil {
1269-
return name.IsConst()
1270-
}
1271-
case *ast.Ident:
1272-
return x.Name == "nil" ||
1273-
strings.HasPrefix(x.Name, "_Ciconst_") ||
1274-
strings.HasPrefix(x.Name, "_Cfconst_") ||
1275-
strings.HasPrefix(x.Name, "_Csconst_") ||
1276-
consts[x.Name]
1277-
case *ast.UnaryExpr:
1278-
return p.isConst(f, x.X)
1279-
case *ast.BinaryExpr:
1280-
return p.isConst(f, x.X) && p.isConst(f, x.Y)
1281-
case *ast.ParenExpr:
1282-
return p.isConst(f, x.X)
1283-
case *ast.CallExpr:
1284-
// Calling the builtin function complex on two untyped
1285-
// constants returns an untyped constant.
1286-
// TODO: It's possible to construct a case that will
1287-
// erroneously succeed if there is a local function
1288-
// named "complex", shadowing the builtin, that returns
1289-
// a numeric type. I can't think of any cases that will
1290-
// erroneously fail.
1291-
if id, ok := x.Fun.(*ast.Ident); ok && id.Name == "complex" && len(x.Args) == 2 {
1292-
return p.isConst(f, x.Args[0]) && p.isConst(f, x.Args[1])
1293-
}
1294-
}
1295-
return false
1296-
}
1297-
12981252
// isVariable reports whether x is a variable, possibly with field references.
12991253
func (p *Package) isVariable(x ast.Expr) bool {
13001254
switch x := x.(type) {

src/cmd/cgo/main.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,6 @@ type File struct {
7171
Edit *edit.Buffer
7272
}
7373

74-
// Untyped constants in the current package.
75-
var consts = make(map[string]bool)
76-
7774
func (f *File) offset(p token.Pos) int {
7875
return fset.Position(p).Offset
7976
}

0 commit comments

Comments
 (0)