Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement warnings for unused imports #16878

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/dmd.warnings-for-unused-imports.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Warnings for unused imports

The D frontend now issues warnings for unused imports if the source is compiled with -w or -wi.

```d
import std.stdio; // source.d(1): Warning: Import `std.stdio` is unused

void main() {}
```

Imports that are added by default by the compiler are not flagged (i.e. object)
2 changes: 1 addition & 1 deletion compiler/src/build.d
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,7 @@ auto sourceFiles()
parse.d parsetimevisitor.d permissivevisitor.d postordervisitor.d pragmasem.d printast.d rootobject.d safe.d
semantic2.d semantic3.d sideeffect.d statement.d statement_rewrite_walker.d
statementsem.d staticassert.d staticcond.d stmtstate.d target.d templatesem.d templateparamsem.d traits.d
transitivevisitor.d typesem.d typinf.d utils.d visitor.d foreachvar.d
transitivevisitor.d typesem.d typinf.d utils.d unused_import.d visitor.d foreachvar.d
cparse.d
"),
backendHeaders: fileArray(env["C"], "
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/cparse.d
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ final class CParser(AST) : Parser!AST
* Import their definitions
*/
auto s = new AST.Import(Loc.initial, null, Id.importc_builtins, null, false);
s.used = true;
wrap.push(s);
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/dimport.d
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ extern (C++) final class Import : Dsymbol
// corresponding AliasDeclarations for alias=name pairs
AliasDeclarations aliasdecls;

bool used;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also import.h


extern (D) this(const ref Loc loc, Identifier[] packages, Identifier id, Identifier aliasId, int isstatic)
{
Identifier selectIdent()
Expand Down
71 changes: 19 additions & 52 deletions compiler/src/dmd/dsymbol.d
Original file line number Diff line number Diff line change
Expand Up @@ -1106,9 +1106,10 @@ extern (C++) class ScopeDsymbol : Dsymbol
Dsymbols* members; // all Dsymbol's in this scope
DsymbolTable symtab; // members[] sorted into table
uint endlinnum; // the linnumber of the statement after the scope (0 if unknown)
/// symbols whose members have been imported, i.e. imported modules and template mixins
Dsymbols* importedScopes;
Visibility.Kind* visibilities; // array of Visibility.Kind, one for each import
/// Hashtable of imported scopes:
// K = imported scopes i.e. the AST node for the imported modules or template mixins
// V = the import AST node or null in case of template mixins
Dsymbol[Dsymbol] importedScopes;

private:

Expand Down Expand Up @@ -1186,57 +1187,21 @@ public:
return os;
}

void importScope(Dsymbol s, Visibility visibility) nothrow
void importScope(Dsymbol s, Visibility visibility, Import imp = null) nothrow
{
//printf("%s.ScopeDsymbol::importScope(%s, %d)\n", toChars(), s.toChars(), visibility);
// No circular or redundant import's
if (s != this)
{
if (!importedScopes)
importedScopes = new Dsymbols();
else
if (s in importedScopes)
{
for (size_t i = 0; i < importedScopes.length; i++)
{
Dsymbol ss = (*importedScopes)[i];
if (ss == s) // if already imported
{
if (visibility.kind > visibilities[i])
visibilities[i] = visibility.kind; // upgrade access
return;
}
}
if (imp && visibility.kind > imp.visibility.kind)
importedScopes[s] = imp;
return;
}
importedScopes.push(s);
visibilities = cast(Visibility.Kind*)mem.xrealloc(visibilities, importedScopes.length * (visibilities[0]).sizeof);
visibilities[importedScopes.length - 1] = visibility.kind;
}
}


/*****************************************
* Returns: the symbols whose members have been imported, i.e. imported modules
* and template mixins.
*
* See_Also: importScope
*/
extern (D) final Dsymbols* getImportedScopes() nothrow @nogc @safe pure
{
return importedScopes;
}

/*****************************************
* Returns: the array of visibilities associated with each imported scope. The
* length of the array matches the imported scopes array.
*
* See_Also: getImportedScopes
*/
extern (D) final Visibility.Kind[] getImportVisibilities() nothrow @nogc @safe pure
{
if (!importedScopes)
return null;

return (() @trusted => visibilities[0 .. importedScopes.length])();
importedScopes[s] = imp ? imp : s;
}
}

extern (D) final void addAccessiblePackage(Package p, Visibility visibility) nothrow
Expand All @@ -1247,16 +1212,18 @@ public:
(*pary)[p.tag] = true;
}

bool isPackageAccessible(Package p, Visibility visibility, SearchOptFlags flags = SearchOpt.all) nothrow
bool isPackageAccessible(Package p, Visibility visibility, SearchOptFlags flags = SearchOpt.all)
{
if (p.tag < accessiblePackages.length && accessiblePackages[p.tag] ||
visibility.kind == Visibility.Kind.private_ && p.tag < privateAccessiblePackages.length && privateAccessiblePackages[p.tag])
return true;
foreach (i, ss; importedScopes ? (*importedScopes)[] : null)
foreach (sym, importer; importedScopes)
{
auto imp = importer.isImport();

// only search visible scopes && imported modules should ignore private imports
if (visibility.kind <= visibilities[i] &&
ss.isScopeDsymbol.isPackageAccessible(p, visibility, SearchOpt.ignorePrivateImports))
if ((!imp || visibility.kind <= imp.visibility.kind) &&
sym.isScopeDsymbol.isPackageAccessible(p, visibility, SearchOpt.ignorePrivateImports))
return true;
}
return false;
Expand Down Expand Up @@ -1536,11 +1503,11 @@ extern (C++) final class ForwardingScopeDsymbol : ScopeDsymbol
return forward.symtabLookup(s,id);
}

override void importScope(Dsymbol s, Visibility visibility)
override void importScope(Dsymbol s, Visibility visibility, Import imp = null)
{
auto forward = parent.isScopeDsymbol();
assert(forward);
forward.importScope(s, visibility);
forward.importScope(s, visibility, imp);
}

override const(char)* kind()const{ return "local scope"; }
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dmd/dsymbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ class ScopeDsymbol : public Dsymbol
Dsymbols *members; // all Dsymbol's in this scope
DsymbolTable *symtab; // members[] sorted into table
unsigned endlinnum; // the linnumber of the statement after the scope (0 if unknown)
Dsymbols *importedScopes; // imported Dsymbol's
Visibility::Kind *visibilities; // array of `Visibility.Kind`, one for each import
void *importedScopes; // hashtable of imported scopes

private:
BitArray accessiblePackages, privateAccessiblePackages;
Expand Down
29 changes: 24 additions & 5 deletions compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,7 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor

if (!imp.isstatic)
{
scopesym.importScope(imp.mod, imp.visibility);
scopesym.importScope(imp.mod, imp.visibility, imp);
}


Expand All @@ -1689,6 +1689,8 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor

sc = sc.push(imp.mod);
sc.visibility = imp.visibility;
if (imp.aliasdecls.length)
imp.used = true;
for (size_t i = 0; i < imp.aliasdecls.length; i++)
{
AliasDeclaration ad = imp.aliasdecls[i];
Expand Down Expand Up @@ -6149,14 +6151,16 @@ private extern(C++) class SearchVisitor : Visitor
//printf(" look in imports\n");
Dsymbol s = null;
OverloadSet a = null;
Import importedFrom = null;
// Look in imported modules
for (size_t i = 0; i < sds.importedScopes.length; i++)
foreach(sym, importer; sds.importedScopes)
{
auto imp = importer.isImport();
// If private import, don't search it
if ((flags & SearchOpt.ignorePrivateImports) && sds.visibilities[i] == Visibility.Kind.private_)
if ((flags & SearchOpt.ignorePrivateImports) && (imp && imp.visibility.kind == Visibility.Kind.private_))
continue;
SearchOptFlags sflags = flags & (SearchOpt.ignoreErrors | SearchOpt.ignoreAmbiguous); // remember these in recursive searches
Dsymbol ss = (*sds.importedScopes)[i];
Dsymbol ss = sym;
//printf("\tscanning import '%s', visibilities = %d, isModule = %p, isImport = %p\n", ss.toChars(), visibilities[i], ss.isModule(), ss.isImport());

if (ss.isModule())
Expand All @@ -6181,6 +6185,7 @@ private extern(C++) class SearchVisitor : Visitor
if (!s)
{
s = s2;
importedFrom = imp;
if (s && s.isOverloadSet())
a = sds.mergeOverloadSet(ident, a, s);
}
Expand All @@ -6194,7 +6199,10 @@ private extern(C++) class SearchVisitor : Visitor
* the other.
*/
if (s.isDeprecated() || s.visible() < s2.visible() && s2.visible().kind != Visibility.Kind.none)
{
s = s2;
importedFrom = imp;
}
}
else
{
Expand Down Expand Up @@ -6225,6 +6233,8 @@ private extern(C++) class SearchVisitor : Visitor
}
if (!symbolIsVisible(sds, s))
s = s2;

importedFrom = imp;
continue;
}

Expand Down Expand Up @@ -6272,6 +6282,13 @@ private extern(C++) class SearchVisitor : Visitor
a = sds.mergeOverloadSet(ident, a, s);
s = a;
}
if (importedFrom)
{
importedFrom.used = true;
//if (sc.module_ && sc.module_.isRoot)
//printf("import %s is used\n", importedFrom.toChars());
}

//printf("\tfound in imports %s.%s\n", toChars(), s.toChars());
return setResult(s);
}
Expand Down Expand Up @@ -6897,7 +6914,7 @@ extern(C++) class ImportAllVisitor : Visitor
if (sc.explicitVisibility)
imp.visibility = sc.visibility;
if (!imp.isstatic && !imp.aliasId && !imp.names.length)
sc.scopesym.importScope(imp.mod, imp.visibility);
sc.scopesym.importScope(imp.mod, imp.visibility, imp);
// Enable access to pkgs/mod as soon as posible, because compiler
// can traverse them before the import gets semantic (Issue: 21501)
if (!imp.aliasId && !imp.names.length)
Expand Down Expand Up @@ -6935,6 +6952,8 @@ extern(C++) class ImportAllVisitor : Visitor
(*m.members)[0].isImport() is null))
{
auto im = new Import(Loc.initial, null, Id.object, null, 0);
// consider it used;
im.used = true;
m.members.shift(im);
}
if (!m.symtab)
Expand Down
Loading