From 83d25c61e45f2d38e1015d6eef50cccc0dc0d9b3 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Tue, 29 Nov 2016 17:27:15 -0800 Subject: [PATCH] go/types: add UsesCgo config to support _cgo_gotypes.go This CL adds a UsesCgo config setting to go/types to specify that the _cgo_gotypes.go file generated by cmd/cgo has been provided as a source file. The type checker then internally resolves C.bar qualified identifiers to _Cfoo_bar as appropriate. It also adds support to srcimporter to automatically run cgo. Unfortunately, this functionality is not compatible with overriding OpenFile, because cmd/cgo and gcc will directly open files. Updates #16623. Updates #35721. Change-Id: I1e1965fe41b765b7a9da3431f2a86cc16025dee2 Reviewed-on: https://go-review.googlesource.com/c/go/+/33677 Run-TryBot: Matthew Dempsky TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- src/go/internal/srcimporter/srcimporter.go | 60 +++++++++++++- .../internal/srcimporter/srcimporter_test.go | 11 +++ src/go/types/api.go | 11 ++- src/go/types/assignments.go | 2 +- src/go/types/call.go | 79 ++++++++++++++++--- src/go/types/check.go | 9 ++- src/go/types/operand.go | 10 +++ src/go/types/package.go | 1 + src/go/types/resolver.go | 5 +- src/go/types/universe.go | 8 +- 10 files changed, 175 insertions(+), 21 deletions(-) diff --git a/src/go/internal/srcimporter/srcimporter.go b/src/go/internal/srcimporter/srcimporter.go index 2a6c274424d15..daef27c8b9dc8 100644 --- a/src/go/internal/srcimporter/srcimporter.go +++ b/src/go/internal/srcimporter/srcimporter.go @@ -14,8 +14,11 @@ import ( "go/token" "go/types" "io" + "io/ioutil" "os" + "os/exec" "path/filepath" + "strings" "sync" ) @@ -115,7 +118,6 @@ func (p *Importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*type var firstHardErr error conf := types.Config{ IgnoreFuncBodies: true, - FakeImportC: true, // continue type-checking after the first error Error: func(err error) { if firstHardErr == nil && !err.(types.Error).Soft { @@ -125,6 +127,21 @@ func (p *Importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*type Importer: p, Sizes: p.sizes, } + if len(bp.CgoFiles) > 0 { + if p.ctxt.OpenFile != nil { + // cgo, gcc, pkg-config, etc. do not support + // build.Context's VFS. + conf.FakeImportC = true + } else { + conf.UsesCgo = true + file, err := p.cgo(bp) + if err != nil { + return nil, err + } + files = append(files, file) + } + } + pkg, err = conf.Check(bp.ImportPath, p.fset, files, nil) if err != nil { // If there was a hard error it is possibly unsafe @@ -181,6 +198,47 @@ func (p *Importer) parseFiles(dir string, filenames []string) ([]*ast.File, erro return files, nil } +func (p *Importer) cgo(bp *build.Package) (*ast.File, error) { + tmpdir, err := ioutil.TempDir("", "srcimporter") + if err != nil { + return nil, err + } + defer os.RemoveAll(tmpdir) + + args := []string{"go", "tool", "cgo", "-objdir", tmpdir} + if bp.Goroot { + switch bp.ImportPath { + case "runtime/cgo": + args = append(args, "-import_runtime_cgo=false", "-import_syscall=false") + case "runtime/race": + args = append(args, "-import_syscall=false") + } + } + args = append(args, "--") + args = append(args, strings.Fields(os.Getenv("CGO_CPPFLAGS"))...) + args = append(args, bp.CgoCPPFLAGS...) + if len(bp.CgoPkgConfig) > 0 { + cmd := exec.Command("pkg-config", append([]string{"--cflags"}, bp.CgoPkgConfig...)...) + out, err := cmd.CombinedOutput() + if err != nil { + return nil, err + } + args = append(args, strings.Fields(string(out))...) + } + args = append(args, "-I", tmpdir) + args = append(args, strings.Fields(os.Getenv("CGO_CFLAGS"))...) + args = append(args, bp.CgoCFLAGS...) + args = append(args, bp.CgoFiles...) + + cmd := exec.Command(args[0], args[1:]...) + cmd.Dir = bp.Dir + if err := cmd.Run(); err != nil { + return nil, err + } + + return parser.ParseFile(p.fset, filepath.Join(tmpdir, "_cgo_gotypes.go"), nil, 0) +} + // context-controlled file system operations func (p *Importer) absPath(path string) (string, error) { diff --git a/src/go/internal/srcimporter/srcimporter_test.go b/src/go/internal/srcimporter/srcimporter_test.go index 56549434d14d3..c456b8e26a782 100644 --- a/src/go/internal/srcimporter/srcimporter_test.go +++ b/src/go/internal/srcimporter/srcimporter_test.go @@ -232,3 +232,14 @@ func TestIssue23092(t *testing.T) { func TestIssue24392(t *testing.T) { testImportPath(t, "go/internal/srcimporter/testdata/issue24392") } + +func TestCgo(t *testing.T) { + testenv.MustHaveGoBuild(t) + testenv.MustHaveCGO(t) + + importer := New(&build.Default, token.NewFileSet(), make(map[string]*types.Package)) + _, err := importer.ImportFrom("./misc/cgo/test", runtime.GOROOT(), 0) + if err != nil { + t.Fatalf("Import failed: %v", err) + } +} diff --git a/src/go/types/api.go b/src/go/types/api.go index 2a21ad0c53f2e..7787b88906e35 100644 --- a/src/go/types/api.go +++ b/src/go/types/api.go @@ -105,6 +105,15 @@ type Config struct { // Do not use casually! FakeImportC bool + // If UsesCgo is set, the type checker expects the + // _cgo_gotypes.go file generated by running cmd/cgo to be + // provided as a package source file. Qualified identifiers + // referring to package C will be resolved to cgo-provided + // declarations within _cgo_gotypes.go. + // + // It is an error to set both FakeImportC and UsesCgo. + UsesCgo bool + // If Error != nil, it is called with each error found // during type checking; err has dynamic type Error. // Secondary errors (for instance, to enumerate all types @@ -281,7 +290,7 @@ func (tv TypeAndValue) IsBuiltin() bool { // nil Value. func (tv TypeAndValue) IsValue() bool { switch tv.mode { - case constant_, variable, mapindex, value, commaok: + case constant_, variable, mapindex, value, commaok, commaerr: return true } return false diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index efa0cbba50270..34a9d7843d943 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -22,7 +22,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) { switch x.mode { case invalid: return // error reported before - case constant_, variable, mapindex, value, commaok: + case constant_, variable, mapindex, value, commaok, commaerr: // ok default: unreachable() diff --git a/src/go/types/call.go b/src/go/types/call.go index 689ef8744c446..1ef9a4057e82f 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -9,6 +9,7 @@ package types import ( "go/ast" "go/token" + "strings" ) func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { @@ -54,6 +55,8 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { default: // function/method call + cgocall := x.mode == cgofunc + sig, _ := x.typ.Underlying().(*Signature) if sig == nil { check.invalidOp(x.pos(), "cannot call non-function %s", x) @@ -74,7 +77,11 @@ func (check *Checker) call(x *operand, e *ast.CallExpr) exprKind { case 0: x.mode = novalue case 1: - x.mode = value + if cgocall { + x.mode = commaerr + } else { + x.mode = value + } x.typ = sig.results.vars[0].typ // unpack tuple default: x.mode = value @@ -192,10 +199,13 @@ func unpack(get getter, n int, allowCommaOk bool) (getter, int, bool) { }, t.Len(), false } - if x0.mode == mapindex || x0.mode == commaok { + if x0.mode == mapindex || x0.mode == commaok || x0.mode == commaerr { // comma-ok value if allowCommaOk { a := [2]Type{x0.typ, Typ[UntypedBool]} + if x0.mode == commaerr { + a[1] = universeError + } return func(x *operand, i int) { x.mode = value x.expr = x0.expr @@ -302,6 +312,17 @@ func (check *Checker) argument(sig *Signature, i int, x *operand, ellipsis token check.assignment(x, typ, context) } +var cgoPrefixes = [...]string{ + "_Ciconst_", + "_Cfconst_", + "_Csconst_", + "_Ctype_", + "_Cvar_", // actually a pointer to the var + "_Cfpvar_fp_", + "_Cfunc_", + "_Cmacro_", // function to evaluate the expanded expression +} + func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { // these must be declared before the "goto Error" statements var ( @@ -322,16 +343,43 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { check.recordUse(ident, pname) pname.used = true pkg := pname.imported - exp := pkg.scope.Lookup(sel) - if exp == nil { - if !pkg.fake { - check.errorf(e.Sel.Pos(), "%s not declared by package %s", sel, pkg.name) + + var exp Object + funcMode := value + if pkg.cgo { + // cgo special cases C.malloc: it's + // rewritten to _CMalloc and does not + // support two-result calls. + if sel == "malloc" { + sel = "_CMalloc" + } else { + funcMode = cgofunc + } + for _, prefix := range cgoPrefixes { + // cgo objects are part of the current package (in file + // _cgo_gotypes.go). Use regular lookup. + _, exp = check.scope.LookupParent(prefix+sel, check.pos) + if exp != nil { + break + } + } + if exp == nil { + check.errorf(e.Sel.Pos(), "%s not declared by package C", sel) + goto Error + } + check.objDecl(exp, nil) + } else { + exp = pkg.scope.Lookup(sel) + if exp == nil { + if !pkg.fake { + check.errorf(e.Sel.Pos(), "%s not declared by package %s", sel, pkg.name) + } + goto Error + } + if !exp.Exported() { + check.errorf(e.Sel.Pos(), "%s not exported by package %s", sel, pkg.name) + // ok to continue } - goto Error - } - if !exp.Exported() { - check.errorf(e.Sel.Pos(), "%s not exported by package %s", sel, pkg.name) - // ok to continue } check.recordUse(e.Sel, exp) @@ -349,9 +397,16 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) { case *Var: x.mode = variable x.typ = exp.typ + if pkg.cgo && strings.HasPrefix(exp.name, "_Cvar_") { + x.typ = x.typ.(*Pointer).base + } case *Func: - x.mode = value + x.mode = funcMode x.typ = exp.typ + if pkg.cgo && strings.HasPrefix(exp.name, "_Cmacro_") { + x.mode = value + x.typ = x.typ.(*Signature).results.vars[0].typ + } case *Builtin: x.mode = builtin x.typ = exp.typ diff --git a/src/go/types/check.go b/src/go/types/check.go index 71d49ad83d583..a94770ffef3ed 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -7,6 +7,7 @@ package types import ( + "errors" "go/ast" "go/constant" "go/token" @@ -247,7 +248,13 @@ func (check *Checker) handleBailout(err *error) { // Files checks the provided files as part of the checker's package. func (check *Checker) Files(files []*ast.File) error { return check.checkFiles(files) } +var errBadCgo = errors.New("cannot use FakeImportC and UsesCgo together") + func (check *Checker) checkFiles(files []*ast.File) (err error) { + if check.conf.FakeImportC && check.conf.UsesCgo { + return errBadCgo + } + defer check.handleBailout(&err) check.initFiles(files) @@ -348,7 +355,7 @@ func (check *Checker) recordCommaOkTypes(x ast.Expr, a [2]Type) { if a[0] == nil || a[1] == nil { return } - assert(isTyped(a[0]) && isTyped(a[1]) && isBoolean(a[1])) + assert(isTyped(a[0]) && isTyped(a[1]) && (isBoolean(a[1]) || a[1] == universeError)) if m := check.Types; m != nil { for { tv := m[x] diff --git a/src/go/types/operand.go b/src/go/types/operand.go index a762ad9bc8bad..eb49e6b1dc349 100644 --- a/src/go/types/operand.go +++ b/src/go/types/operand.go @@ -27,6 +27,8 @@ const ( mapindex // operand is a map index expression (acts like a variable on lhs, commaok on rhs of an assignment) value // operand is a computed value commaok // like value, but operand may be used in a comma,ok expression + commaerr // like commaok, but second value is error, not boolean + cgofunc // operand is a cgo function ) var operandModeString = [...]string{ @@ -39,6 +41,8 @@ var operandModeString = [...]string{ mapindex: "map index expression", value: "value", commaok: "comma, ok expression", + commaerr: "comma, error expression", + cgofunc: "cgo function", } // An operand represents an intermediate value during type checking. @@ -94,6 +98,12 @@ func (x *operand) pos() token.Pos { // commaok ( ) // commaok ( of type ) // +// commaerr ( ) +// commaerr ( of type ) +// +// cgofunc ( ) +// cgofunc ( of type ) +// func operandString(x *operand, qf Qualifier) string { var buf bytes.Buffer diff --git a/src/go/types/package.go b/src/go/types/package.go index cd202a0ed9f78..7b89def1b5d51 100644 --- a/src/go/types/package.go +++ b/src/go/types/package.go @@ -17,6 +17,7 @@ type Package struct { complete bool imports []*Package fake bool // scope lookup errors are silently dropped if package is fake (internal use only) + cgo bool // uses of this package will be rewritten into uses of declarations from _cgo_gotypes.go } // NewPackage returns a new Package for the given package path and name. diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go index 839d076e36144..f80b4ec7841f1 100644 --- a/src/go/types/resolver.go +++ b/src/go/types/resolver.go @@ -141,9 +141,10 @@ func (check *Checker) importPackage(pos token.Pos, path, dir string) *Package { } // no package yet => import it - if path == "C" && check.conf.FakeImportC { + if path == "C" && (check.conf.FakeImportC || check.conf.UsesCgo) { imp = NewPackage("C", "C") - imp.fake = true + imp.fake = true // package scope is not populated + imp.cgo = check.conf.UsesCgo } else { // ordinary import var err error diff --git a/src/go/types/universe.go b/src/go/types/universe.go index 7af6dab320edd..ff5b89118ad9e 100644 --- a/src/go/types/universe.go +++ b/src/go/types/universe.go @@ -21,9 +21,10 @@ var Universe *Scope var Unsafe *Package var ( - universeIota *Const - universeByte *Basic // uint8 alias, but has name "byte" - universeRune *Basic // int32 alias, but has name "rune" + universeIota *Const + universeByte *Basic // uint8 alias, but has name "byte" + universeRune *Basic // int32 alias, but has name "rune" + universeError *Named ) // Typ contains the predeclared *Basic types indexed by their @@ -200,6 +201,7 @@ func init() { universeIota = Universe.Lookup("iota").(*Const) universeByte = Universe.Lookup("byte").(*TypeName).typ.(*Basic) universeRune = Universe.Lookup("rune").(*TypeName).typ.(*Basic) + universeError = Universe.Lookup("error").(*TypeName).typ.(*Named) } // Objects with names containing blanks are internal and not entered into