diff --git a/src/cmd/compile/internal/types2/check.go b/src/cmd/compile/internal/types2/check.go index 6ba8506916..e2c6c4f606 100644 --- a/src/cmd/compile/internal/types2/check.go +++ b/src/cmd/compile/internal/types2/check.go @@ -74,6 +74,12 @@ type importKey struct { path, dir string } +// A dotImportKey describes a dot-imported object in the given scope. +type dotImportKey struct { + scope *Scope + obj Object +} + // A Checker maintains the state of the type checker. // It must be created with NewChecker. type Checker struct { @@ -92,8 +98,9 @@ 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 []*syntax.File // package files - unusedDotImports map[*Scope]map[*Package]syntax.Pos // positions of unused dot-imported packages for each file scope + files []*syntax.File // list of package files + imports []*PkgName // list of imported packages + dotImportMap map[dotImportKey]*PkgName // maps dot-imported objects to the package they were dot-imported through firstErr error // first error encountered methods map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods @@ -110,22 +117,6 @@ 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 syntax.Pos) { - mm := check.unusedDotImports - if mm == nil { - mm = make(map[*Scope]map[*Package]syntax.Pos) - check.unusedDotImports = mm - } - m := mm[scope] - if m == nil { - m = make(map[*Package]syntax.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 @@ -209,7 +200,8 @@ func NewChecker(conf *Config, pkg *Package, info *Info) *Checker { func (check *Checker) initFiles(files []*syntax.File) { // start with a clean slate (check.Files may be called multiple times) check.files = nil - check.unusedDotImports = nil + check.imports = nil + check.dotImportMap = nil check.firstErr = nil check.methods = nil @@ -291,6 +283,9 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) { print("== unusedImports ==") check.unusedImports() } + // no longer needed - release memory + check.imports = nil + check.dotImportMap = nil print("== recordUntyped ==") check.recordUntyped() @@ -301,6 +296,9 @@ func (check *Checker) checkFiles(files []*syntax.File) (err error) { } check.pkg.complete = true + + // TODO(gri) There's more memory we should release at this point. + return } diff --git a/src/cmd/compile/internal/types2/resolver.go b/src/cmd/compile/internal/types2/resolver.go index 7ea9bde5fa..2a84015cfc 100644 --- a/src/cmd/compile/internal/types2/resolver.go +++ b/src/cmd/compile/internal/types2/resolver.go @@ -273,21 +273,26 @@ func (check *Checker) collectObjects() { } } - obj := NewPkgName(s.Pos(), pkg, name, imp) + pkgName := NewPkgName(s.Pos(), pkg, name, imp) if s.LocalPkgName != nil { // in a dot-import, the dot represents the package - check.recordDef(s.LocalPkgName, obj) + check.recordDef(s.LocalPkgName, pkgName) } else { - check.recordImplicit(s, obj) + check.recordImplicit(s, pkgName) } if path == "C" { // match cmd/compile (not prescribed by spec) - obj.used = true + pkgName.used = true } // add import to file scope + check.imports = append(check.imports, pkgName) if name == "." { + // dot-import + if check.dotImportMap == nil { + check.dotImportMap = make(map[dotImportKey]*PkgName) + } // merge imported scope with file scope for _, obj := range imp.scope.elems { // A package scope may contain non-exported objects, @@ -301,16 +306,15 @@ func (check *Checker) collectObjects() { if alt := fileScope.Insert(obj); alt != nil { check.errorf(s.LocalPkgName, "%s redeclared in this block", obj.Name()) check.reportAltDecl(alt) + } else { + check.dotImportMap[dotImportKey{fileScope, obj}] = pkgName } } } - // add position to set of dot-import positions for this file - // (this is only needed for "imported but not used" errors) - check.addUnusedDotImport(fileScope, imp, s.Pos()) } else { // declare imported package object in file scope // (no need to provide s.LocalPkgName since we called check.recordDef earlier) - check.declare(fileScope, nil, obj, nopos) + check.declare(fileScope, nil, pkgName, nopos) } case *syntax.ConstDecl: @@ -673,51 +677,38 @@ func (check *Checker) unusedImports() { // any of its exported identifiers. To import a package solely for its side-effects // (initialization), use the blank identifier as explicit package name." - // check use of regular imported packages - for _, scope := range check.pkg.scope.children /* file scopes */ { - for _, obj := range scope.elems { - if obj, ok := obj.(*PkgName); ok { - // Unused "blank imports" are automatically ignored - // since _ identifiers are not entered into scopes. - if !obj.used { - path := obj.imported.path - base := pkgName(path) - if obj.name == base { - if check.conf.CompilerErrorMessages { - check.softErrorf(obj.pos, "%q imported and not used", path) - } else { - check.softErrorf(obj.pos, "%q imported but not used", path) - } - } else { - if check.conf.CompilerErrorMessages { - check.softErrorf(obj.pos, "%q imported and not used as %s", path, obj.name) - } else { - check.softErrorf(obj.pos, "%q imported but not used as %s", path, obj.name) - } - } - } - } - } - } - - // check use of dot-imported packages - for _, unusedDotImports := range check.unusedDotImports { - for pkg, pos := range unusedDotImports { - if check.conf.CompilerErrorMessages { - check.softErrorf(pos, "%q imported and not used", pkg.path) - } else { - check.softErrorf(pos, "%q imported but not used", pkg.path) - } + for _, obj := range check.imports { + if !obj.used && obj.name != "_" { + check.errorUnusedPkg(obj) } } } -// pkgName returns the package name (last element) of an import path. -func pkgName(path string) string { - if i := strings.LastIndex(path, "/"); i >= 0 { - path = path[i+1:] +func (check *Checker) errorUnusedPkg(obj *PkgName) { + // If the package was imported with a name other than the final + // import path element, show it explicitly in the error message. + // Note that this handles both renamed imports and imports of + // packages containing unconventional package declarations. + // Note that this uses / always, even on Windows, because Go import + // paths always use forward slashes. + path := obj.imported.path + elem := path + if i := strings.LastIndex(elem, "/"); i >= 0 { + elem = elem[i+1:] + } + if obj.name == "" || obj.name == "." || obj.name == elem { + if check.conf.CompilerErrorMessages { + check.softErrorf(obj, "imported and not used: %q", path) + } else { + check.softErrorf(obj, "%q imported but not used", path) + } + } else { + if check.conf.CompilerErrorMessages { + check.softErrorf(obj, "imported and not used: %q as %s", path, obj.name) + } else { + check.softErrorf(obj, "%q imported but not used as %s", path, obj.name) + } } - return path } // dir makes a good-faith attempt to return the directory diff --git a/src/cmd/compile/internal/types2/testdata/importdecl0/importdecl0b.src b/src/cmd/compile/internal/types2/testdata/importdecl0/importdecl0b.src index 48ecb5e46f..19b55aff76 100644 --- a/src/cmd/compile/internal/types2/testdata/importdecl0/importdecl0b.src +++ b/src/cmd/compile/internal/types2/testdata/importdecl0/importdecl0b.src @@ -8,7 +8,7 @@ import "math" import m "math" import . "testing" // declares T in file scope -import . /* ERROR "imported but not used" */ "unsafe" +import . /* ERROR .unsafe. imported but not used */ "unsafe" import . "fmt" // declares Println in file scope import ( diff --git a/src/cmd/compile/internal/types2/testdata/importdecl1/importdecl1b.src b/src/cmd/compile/internal/types2/testdata/importdecl1/importdecl1b.src index ee70bbd8e7..43a7bcd753 100644 --- a/src/cmd/compile/internal/types2/testdata/importdecl1/importdecl1b.src +++ b/src/cmd/compile/internal/types2/testdata/importdecl1/importdecl1b.src @@ -4,7 +4,7 @@ package importdecl1 -import . /* ERROR "imported but not used" */ "unsafe" +import . /* ERROR .unsafe. imported but not used */ "unsafe" type B interface { A diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 9ab84b594b..b758c0f358 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -56,12 +56,12 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *Named, wantType boo } 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. + // The object may have been dot-imported. + // If so, mark the respective package as used. // (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) + if pkgName := check.dotImportMap[dotImportKey{scope, obj}]; pkgName != nil { + pkgName.used = true } switch obj := obj.(type) { diff --git a/test/run.go b/test/run.go index a1c68494c3..8b487aa76f 100644 --- a/test/run.go +++ b/test/run.go @@ -1956,7 +1956,6 @@ var excluded = map[string]bool{ "fixedbugs/issue18331.go": true, // missing error about misuse of //go:noescape (irgen needs code from noder) "fixedbugs/issue18393.go": true, // types2 not run after syntax errors "fixedbugs/issue19012.go": true, // multiple errors on same line - "fixedbugs/issue20298.go": true, // types2 non-deterministically reports unused imports "fixedbugs/issue20233.go": true, // types2 reports two instead of one error (pref: compiler) "fixedbugs/issue20245.go": true, // types2 reports two instead of one error (pref: compiler) "fixedbugs/issue20250.go": true, // correct diagnostics, but different lines (probably irgen's fault)