qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 17:04:41 -07:00
Code Releases Activity

go/types: don't mark external package objects as used

Browse Source 
Also removes a potential race condition regarding the
used flag of Var objects when type-checking packages
concurrently.

Implementation: Rather than marking all used dot-imported
objects and then deduce which corresponding package was used,
now we consider all dot-imported packages as unused and remove
each package from the unused packages map as objects are used.

Now only objects that can be marked as used have a used field
(variables, labels, and packages).

As a result, the code became cleaner and simpler.

Fixes golang/go#8969.

LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/163740043
This commit is contained in:
Robert Griesemer 2014-10-23 09:39:20 -07:00
parent b916c559e7
commit b10dfb99c5
12 changed files with 105 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
go/types/assignments.go
View File

@ -161,7 +161,7 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
var v *Var
var v_used bool
if ident != nil {
if obj := check.scope.LookupParent(ident.Name); obj != nil {
if _, obj := check.scope.LookupParent(ident.Name); obj != nil {
v, _ = obj.(*Var)
if v != nil {
v_used = v.used

4
go/types/call.go
View File

@ -266,7 +266,9 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) {
// can only appear in qualified identifiers which are mapped to
// selector expressions.
if ident, ok := e.X.(*ast.Ident); ok {
if pkg, _ := check.scope.LookupParent(ident.Name).(*PkgName); pkg != nil {
_, obj := check.scope.LookupParent(ident.Name)
if pkg, _ := obj.(*PkgName); pkg != nil {
assert(pkg.pkg == check.pkg)
check.recordUse(ident, pkg)
pkg.used = true
exp := pkg.imported.scope.Lookup(sel)

39
go/types/check.go
View File

@ -7,7 +7,6 @@
package types
import (
"fmt"
"go/ast"
"go/token"
@ -73,9 +72,8 @@ type Checker struct {
// information collected during type-checking of a set of package files
// (initialized by Files, valid only for the duration of check.Files;
// maps and lists are allocated on demand)
files []*ast.File // package files
fileScopes []*Scope // file scope for each file
dotImports []map[*Package]token.Pos // positions of dot-imports for each file
files []*ast.File // package files
unusedDotImports map[*Scope]map[*Package]token.Pos // positions of unused dot-imported packages for each file scope
firstErr error // first error encountered
methods map[string][]*Func // maps type names to associated methods
@ -91,6 +89,22 @@ type Checker struct {
indent int // indentation for tracing
}
// addUnusedImport adds the position of a dot-imported package
// pkg to the map of dot imports for the given file scope.
func (check *Checker) addUnusedDotImport(scope *Scope, pkg *Package, pos token.Pos) {
mm := check.unusedDotImports
if mm == nil {
mm = make(map[*Scope]map[*Package]token.Pos)
check.unusedDotImports = mm
}
m := mm[scope]
if m == nil {
m = make(map[*Package]token.Pos)
mm[scope] = m
}
m[pkg] = pos
}
// addDeclDep adds the dependency edge (check.decl -> to) if check.decl exists
func (check *Checker) addDeclDep(to Object) {
from := check.decl
@ -161,8 +175,7 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Ch
func (check *Checker) initFiles(files []*ast.File) {
// start with a clean slate (check.Files may be called multiple times)
check.files = nil
check.fileScopes = nil
check.dotImports = nil
check.unusedDotImports = nil
check.firstErr = nil
check.methods = nil
@ -170,9 +183,9 @@ func (check *Checker) initFiles(files []*ast.File) {
check.funcs = nil
check.delayed = nil
// determine package name, files, and set up file scopes, dotImports maps
// determine package name and collect valid files
pkg := check.pkg
for i, file := range files {
for _, file := range files {
switch name := file.Name.Name; pkg.name {
case "":
if name != "_" {
@ -184,16 +197,6 @@ func (check *Checker) initFiles(files []*ast.File) {
case name:
check.files = append(check.files, file)
var comment string
if pos := file.Pos(); pos.IsValid() {
comment = "file " + check.fset.File(pos).Name()
} else {
comment = fmt.Sprintf("file[%d]", i)
}
fileScope := NewScope(pkg.scope, comment)
check.recordScope(file, fileScope)
check.fileScopes = append(check.fileScopes, fileScope)
check.dotImports = append(check.dotImports, nil) // element (map) is lazily allocated
default:
check.errorf(file.Package, "package %s; expected %s", name, pkg.name)

1
go/types/check_test.go
View File

@ -53,6 +53,7 @@ var (
var tests = [][]string{
{"testdata/errors.src"},
{"testdata/importdecl0a.src", "testdata/importdecl0b.src"},
{"testdata/importdecl1a.src", "testdata/importdecl1b.src"},
{"testdata/cycles.src"},
{"testdata/cycles1.src"},
{"testdata/cycles2.src"},

33
go/types/object.go
View File

@ -37,9 +37,6 @@ type Object interface {
// 0 for all other objects (including objects in file scopes).
order() uint32
// isUsed reports whether the object was marked as 'used'.
isUsed() bool
// setOrder sets the order number of the object. It must be > 0.
setOrder(uint32)
@ -82,9 +79,7 @@ type object struct {
pkg *Package
name string
typ Type
order_ uint32
used bool
}
func (obj *object) Parent() *Scope { return obj.parent }
@ -95,9 +90,7 @@ func (obj *object) Type() Type { return obj.typ }
func (obj *object) Exported() bool { return ast.IsExported(obj.name) }
func (obj *object) Id() string { return Id(obj.pkg, obj.name) }
func (obj *object) String() string { panic("abstract") }
func (obj *object) order() uint32 { return obj.order_ }
func (obj *object) isUsed() bool { return obj.used }
func (obj *object) order() uint32 { return obj.order_ }
func (obj *object) setOrder(order uint32) { assert(order > 0); obj.order_ = order }
func (obj *object) setParent(parent *Scope) { obj.parent = parent }
@ -128,10 +121,11 @@ func (obj *object) sameId(pkg *Package, name string) bool {
type PkgName struct {
object
imported *Package
used bool // set if the package was used
}
func NewPkgName(pos token.Pos, pkg *Package, name string, imported *Package) *PkgName {
return &PkgName{object{nil, pos, pkg, name, Typ[Invalid], 0, false}, imported}
return &PkgName{object{nil, pos, pkg, name, Typ[Invalid], 0}, imported, false}
}
// Imported returns the package that was imported.
@ -141,13 +135,12 @@ func (obj *PkgName) Imported() *Package { return obj.imported }
// A Const represents a declared constant.
type Const struct {
object
val exact.Value
val exact.Value
visited bool // for initialization cycle detection
}
func NewConst(pos token.Pos, pkg *Package, name string, typ Type, val exact.Value) *Const {
return &Const{object: object{nil, pos, pkg, name, typ, 0, false}, val: val}
return &Const{object{nil, pos, pkg, name, typ, 0}, val, false}
}
func (obj *Const) Val() exact.Value { return obj.val }
@ -158,28 +151,28 @@ type TypeName struct {
}
func NewTypeName(pos token.Pos, pkg *Package, name string, typ Type) *TypeName {
return &TypeName{object{nil, pos, pkg, name, typ, 0, false}}
return &TypeName{object{nil, pos, pkg, name, typ, 0}}
}
// A Variable represents a declared variable (including function parameters and results, and struct fields).
type Var struct {
object
anonymous bool // if set, the variable is an anonymous struct field, and name is the type name
visited bool // for initialization cycle detection
isField bool // var is struct field
used bool // set if the variable was used
}
func NewVar(pos token.Pos, pkg *Package, name string, typ Type) *Var {
return &Var{object: object{nil, pos, pkg, name, typ, 0, false}}
return &Var{object: object{nil, pos, pkg, name, typ, 0}}
}
func NewParam(pos token.Pos, pkg *Package, name string, typ Type) *Var {
return &Var{object: object{nil, pos, pkg, name, typ, 0, true}} // parameters are always 'used'
return &Var{object: object{nil, pos, pkg, name, typ, 0}, used: true} // parameters are always 'used'
}
func NewField(pos token.Pos, pkg *Package, name string, typ Type, anonymous bool) *Var {
return &Var{object: object{nil, pos, pkg, name, typ, 0, false}, anonymous: anonymous, isField: true}
return &Var{object: object{nil, pos, pkg, name, typ, 0}, anonymous: anonymous, isField: true}
}
func (obj *Var) Anonymous() bool { return obj.anonymous }
@ -199,7 +192,7 @@ func NewFunc(pos token.Pos, pkg *Package, name string, sig *Signature) *Func {
if sig != nil {
typ = sig
}
return &Func{object{nil, pos, pkg, name, typ, 0, false}}
return &Func{object{nil, pos, pkg, name, typ, 0}}
}
// FullName returns the package- or receiver-type-qualified name of
@ -217,17 +210,17 @@ func (obj *Func) Scope() *Scope {
// A Label represents a declared label.
type Label struct {
object
used bool // set if the label was used
}
func NewLabel(pos token.Pos, pkg *Package, name string) *Label {
return &Label{object{pos: pos, pkg: pkg, name: name, typ: Typ[Invalid]}}
return &Label{object{pos: pos, pkg: pkg, name: name, typ: Typ[Invalid]}, false}
}
// A Builtin represents a built-in function.
// Builtins don't have a valid type.
type Builtin struct {
object
id builtinId
}

54
go/types/resolver.go
View File

@ -113,6 +113,15 @@ func (check *Checker) declarePkgObj(ident *ast.Ident, obj Object, d *declInfo) {
obj.setOrder(uint32(len(check.objMap)))
}
// filename returns a filename suitable for debugging output.
func (check *Checker) filename(fileNo int) string {
file := check.files[fileNo]
if pos := file.Pos(); pos.IsValid() {
return check.fset.File(pos).Name()
}
return fmt.Sprintf("file[%d]", fileNo)
}
// collectObjects collects all file and package objects and inserts them
// into their respective scopes. It also performs imports and associates
// methods with receiver base type names.
@ -139,7 +148,9 @@ func (check *Checker) collectObjects() {
// The package identifier denotes the current package,
// but there is no corresponding package object.
check.recordDef(file.Name, nil)
fileScope := check.fileScopes[fileNo]
fileScope := NewScope(check.pkg.scope, check.filename(fileNo))
check.recordScope(file, fileScope)
for _, decl := range file.Decls {
switch d := decl.(type) {
@ -223,12 +234,7 @@ func (check *Checker) collectObjects() {
}
// add position to set of dot-import positions for this file
// (this is only needed for "imported but not used" errors)
posSet := check.dotImports[fileNo]
if posSet == nil {
posSet = make(map[*Package]token.Pos)
check.dotImports[fileNo] = posSet
}
posSet[imp] = s.Pos()
check.addUnusedDotImport(fileScope, imp, s.Pos())
} else {
// declare imported package object in file scope
check.declare(fileScope, nil, obj)
@ -351,7 +357,7 @@ func (check *Checker) collectObjects() {
}
// verify that objects in package and file scopes have different names
for _, scope := range check.fileScopes {
for _, scope := range check.pkg.scope.children /* file scopes */ {
for _, obj := range scope.elems {
if alt := pkg.scope.Lookup(obj.Name()); alt != nil {
if pkg, ok := obj.(*PkgName); ok {
@ -407,11 +413,11 @@ func (check *Checker) unusedImports() {
// spec: "It is illegal (...) to directly import a package without referring to
// any of its exported identifiers. To import a package solely for its side-effects
// (initialization), use the blank identifier as explicit package name."
for i, scope := range check.fileScopes {
var usedDotImports map[*Package]bool // lazily allocated
// check use of regular imported packages
for _, scope := range check.pkg.scope.children /* file scopes */ {
for _, obj := range scope.elems {
switch obj := obj.(type) {
case *PkgName:
if obj, ok := obj.(*PkgName); ok {
// Unused "blank imports" are automatically ignored
// since _ identifiers are not entered into scopes.
if !obj.used {
@ -423,24 +429,14 @@ func (check *Checker) unusedImports() {
check.softErrorf(obj.pos, "%q imported but not used as %s", path, obj.name)
}
}
default:
// All other objects in the file scope must be dot-
// imported. If an object was used, mark its package
// as used.
if obj.isUsed() {
if usedDotImports == nil {
usedDotImports = make(map[*Package]bool)
}
usedDotImports[obj.Pkg()] = true
}
}
}
// Iterate through all dot-imports for this file and
// check if the corresponding package was used.
for pkg, pos := range check.dotImports[i] {
if !usedDotImports[pkg] {
check.softErrorf(pos, "%q imported but not used", pkg.path)
}
}
}
// check use of dot-imported packages
for _, unusedDotImports := range check.unusedDotImports {
for pkg, pos := range unusedDotImports {
check.softErrorf(pos, "%q imported but not used", pkg.path)
}
}
}

2
go/types/return.go
View File

@ -31,7 +31,7 @@ func (check *Checker) isTerminating(s ast.Stmt, label string) bool {
// the predeclared (possibly parenthesized) panic() function is terminating
if call, _ := unparen(s.X).(*ast.CallExpr); call != nil {
if id, _ := call.Fun.(*ast.Ident); id != nil {
if obj := check.scope.LookupParent(id.Name); obj != nil {
if _, obj := check.scope.LookupParent(id.Name); obj != nil {
if b, _ := obj.(*Builtin); b != nil && b.id == _Panic {
return true
}

13
go/types/scope.go
View File

@ -71,14 +71,19 @@ func (s *Scope) Lookup(name string) Object {
// LookupParent follows the parent chain of scopes starting with s until
// it finds a scope where Lookup(name) returns a non-nil object, and then
// returns that object. If no such scope exists, the result is nil.
func (s *Scope) LookupParent(name string) Object {
// returns that scope and object. If no such scope exists, the result is (nil, nil).
//
// Note that obj.Parent() may be different from the returned scope if the
// object was inserted into the scope and already had a parent at that
// time (see Insert, below). This can only happen for dot-imported objects
// whose scope is the scope of the package that exported them.
func (s *Scope) LookupParent(name string) (*Scope, Object) {
for ; s != nil; s = s.parent {
if obj := s.elems[name]; obj != nil {
return obj
return s, obj
}
}
return nil
return nil, nil
}
// Insert attempts to insert an object obj into scope s.

2
go/types/stmt.go
View File

@ -329,7 +329,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
// list in a "return" statement if a different entity (constant, type, or variable)
// with the same name as a result parameter is in scope at the place of the return."
for _, obj := range res.vars {
if alt := check.scope.LookupParent(obj.name); alt != nil && alt != obj {
if _, alt := check.scope.LookupParent(obj.name); alt != nil && alt != obj {
check.errorf(s.Pos(), "result parameter %s not in scope at return", obj.name)
check.errorf(alt.Pos(), "\tinner declaration of %s", obj)
// ok to continue

11
go/types/testdata/importdecl1a.src vendored Normal file
View File

@ -0,0 +1,11 @@
// Copyright 2014 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.
// Test case for issue 8969.
package importdecl1
import . "unsafe"
var _ Pointer // use dot-imported package unsafe

7
go/types/testdata/importdecl1b.src vendored Normal file
View File

@ -0,0 +1,7 @@
// Copyright 2014 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 importdecl1
import . /* ERROR "imported but not used" */ "unsafe"

24
go/types/typexpr.go
View File

@ -23,7 +23,7 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa
x.mode = invalid
x.expr = e
obj := check.scope.LookupParent(e.Name)
scope, obj := check.scope.LookupParent(e.Name)
if obj == nil {
if e.Name == "_" {
check.errorf(e.Pos(), "cannot use _ as value or type")
@ -38,18 +38,20 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa
typ := obj.Type()
assert(typ != nil)
// The object may be dot-imported: If so, remove its package from
// the map of unused dot imports for the respective file scope.
// (This code is only needed for dot-imports. Without them,
// we only have to mark variables, see *Var case below).
if pkg := obj.Pkg(); pkg != check.pkg && pkg != nil {
delete(check.unusedDotImports[scope], pkg)
}
switch obj := obj.(type) {
case *PkgName:
check.errorf(e.Pos(), "use of package %s not in selector", obj.name)
return
case *Const:
// The constant may be dot-imported. Mark it as used so that
// later we can determine if the corresponding dot-imported
// package was used. Same applies for other objects, below.
// (This code is only used for dot-imports. Without them, we
// would only have to mark Vars.)
obj.used = true
check.addDeclDep(obj)
if typ == Typ[Invalid] {
return
@ -67,7 +69,6 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa
x.mode = constant
case *TypeName:
obj.used = true
x.mode = typexpr
// check for cycle
// (it's ok to iterate forward because each named type appears at most once in path)
@ -86,7 +87,9 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa
}
case *Var:
obj.used = true
if obj.pkg == check.pkg {
obj.used = true
}
check.addDeclDep(obj)
if typ == Typ[Invalid] {
return
@ -94,17 +97,14 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa
x.mode = variable
case *Func:
obj.used = true
check.addDeclDep(obj)
x.mode = value
case *Builtin:
obj.used = true // for built-ins defined by package unsafe
x.id = obj.id
x.mode = builtin
case *Nil:
// no need to "use" the nil object
x.mode = value
default: