Skip to content

Commit

Permalink
fix Issue 10378 - Local imports hide local symbols
Browse files Browse the repository at this point in the history
  • Loading branch information
@WalterBright
WalterBright committed Feb 15, 2016
1 parent 3d726b1 commit e5be073
Show file tree
Hide file tree
Showing 20 changed files with 303 additions and 65 deletions.
25 changes: 25 additions & 0 deletions changelog.dd
View file
Expand Up @@ -9,6 +9,7 @@ $(BUGSTITLE Compiler Changes,

$(BUGSTITLE Language Changes,
$(LI $(RELATIVE_LINK2 extended-deprecated, Manifest constant can now be used for deprecation message.))
$(LI $(RELATIVE_LINK2 import-lookup, Imports no longer hide locals declared in outer scopes.))
)

$(BUGSTITLE Compiler Changes,
Expand All @@ -28,6 +29,30 @@ $(BUGSTITLE Language Changes,
deprecated("Some long deprecation " ~ "message") class Bar {}
---
)

$(LI $(LNAME2 import-lookup, Imports no longer hide locals declared in outer scopes.))

These changes were made to the name lookup algorithm:

$(OL
$(LI Lookup for unqualified names is change from one pass to two pass. The first
pass goes through the scopes but does not check import declarations. If not found,
the second pass goes through the scopes and only looks at import declarations.
)
$(LI Qualified name lookups, base class lookups, and WithStatement lookups no longer
search import declarations, unless a module is the subject of the lookup, where the
behavior remains as before.
)
)

$(P This can break existing code, although reliance on the previous behavior tends to be
unintended, and fixing it improves the comprehensibility of the code. Breakage tends
to take the form of a symbol now being flagged as undefined. Fixing the breakage can
be done by fully qualifying the name, or adding an alias to the import declaration.)

$(P Restore old behavior using the -transition=import compiler switch.)

$(P See also $(BUGZILLA 10378))
)

Macros:
Expand Down
8 changes: 4 additions & 4 deletions src/dclass.d
View file
Expand Up @@ -1076,9 +1076,9 @@ public:
return baseok >= BASEOKdone;
}

override final Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override final Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
//printf("%s.ClassDeclaration.search('%s')\n", toChars(), ident.toChars());
//printf("%s.ClassDeclaration.search('%s', flags=x%x)\n", toChars(), ident.toChars(), flags);
//if (scope) printf("%s baseok = %d\n", toChars(), baseok);
if (_scope && baseok < BASEOKdone)
{
Expand All @@ -1098,7 +1098,7 @@ public:
return null;
}

Dsymbol s = ScopeDsymbol.search(loc, ident, flags);
Dsymbol s = ScopeDsymbol.search(loc, ident, (flags & SearchImportsOnly) ? flags : flags | SearchLocalsOnly);
if (!s)
{
// Search bases classes in depth-first, left to right order
Expand All @@ -1111,7 +1111,7 @@ public:
error("base %s is forward referenced", b.sym.ident.toChars());
else
{
s = b.sym.search(loc, ident, flags);
s = b.sym.search(loc, ident, flags | SearchLocalsOnly);
if (s == this) // happens if s is nested in this and derives from this
s = null;
else if (s)
Expand Down
2 changes: 1 addition & 1 deletion src/declaration.d
View file
Expand Up @@ -234,7 +234,7 @@ public:
return 1;
}

override final Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override final Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
Dsymbol s = Dsymbol.search(loc, ident, flags);
if (!s && type)
Expand Down
2 changes: 1 addition & 1 deletion src/denum.d
View file
Expand Up @@ -308,7 +308,7 @@ public:
return "enum";
}

override Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
//printf("%s.EnumDeclaration::search('%s')\n", toChars(), ident->toChars());
if (_scope)
Expand Down
4 changes: 2 additions & 2 deletions src/dimport.d
View file
Expand Up @@ -443,9 +443,9 @@ public:
}
}

override Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
//printf("%s.Import::search(ident = '%s', flags = x%x)\n", toChars(), ident.toChars(), flags);
//printf("%s.Import.search(ident = '%s', flags = x%x)\n", toChars(), ident.toChars(), flags);
if (!pkg)
{
load(null);
Expand Down
15 changes: 12 additions & 3 deletions src/dmodule.d
View file
Expand Up @@ -222,8 +222,10 @@ public:
{
}

override Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
//printf("%s Package.search('%s', flags = x%x)\n", toChars(), ident.toChars(), flags);
flags &= ~SearchLocalsOnly; // searching an import is always transitive
if (!isModule() && mod)
{
// Prefer full package name.
Expand Down Expand Up @@ -1068,15 +1070,22 @@ public:
return needmoduleinfo || global.params.cov;
}

override Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
/* Since modules can be circularly referenced,
* need to stop infinite recursive searches.
* This is done with the cache.
*/
//printf("%s Module::search('%s', flags = %d) insearch = %d\n", toChars(), ident->toChars(), flags, insearch);
//printf("%s Module.search('%s', flags = x%x) insearch = %d\n", toChars(), ident.toChars(), flags, insearch);
if (insearch)
return null;

/* Qualified module searches always search their imports,
* even if SearchLocalsOnly
*/
if (!(flags & SearchUnqualifiedModule))
flags &= ~(SearchUnqualifiedModule | SearchLocalsOnly);

if (searchCacheIdent == ident && searchCacheFlags == flags)
{
//printf("%s Module::search('%s', flags = %d) insearch = %d searchCacheSymbol = %s\n",
Expand Down
104 changes: 89 additions & 15 deletions src/dscope.d
View file
Expand Up @@ -32,6 +32,8 @@ import ddmd.root.speller;
import ddmd.root.stringtable;
import ddmd.statement;

//version=LOGSEARCH;

extern (C++) bool mergeFieldInit(Loc loc, ref uint fieldInit, uint fi, bool mustInit)
{
if (fi != fieldInit)
Expand Down Expand Up @@ -419,9 +421,44 @@ struct Scope
return minst ? minst : _module;
}

/************************************
* Perform unqualified name lookup by following the chain of scopes up
* until found.
*
* Params:
* loc = location to use for error messages
* ident = name to look up
* pscopesym = if supplied and name is found, set to scope that ident was found in
* flags = modify search based on flags
*
* Returns:
* symbol if found, null if not
*/
extern (C++) Dsymbol search(Loc loc, Identifier ident, Dsymbol* pscopesym, int flags = IgnoreNone)
{
//printf("Scope::search(%p, '%s')\n", this, ident->toChars());
version (LOGSEARCH)
{
printf("Scope.search(%p, '%s' flags=x%x)\n", &this, ident.toChars(), flags);
// Print scope chain
for (Scope* sc = &this; sc; sc = sc.enclosing)
{
if (!sc.scopesym)
continue;
printf("\tscope %s\n", sc.scopesym.toChars());
}

static void printMsg(string txt, Dsymbol s)
{
printf("%.*s %s.%s, kind = '%s'\n", cast(int)msg.length, msg.ptr,
s.parent ? s.parent.toChars() : "", s.toChars(), s.kind());
}
}

// This function is called only for unqualified lookup
assert(!(flags & (SearchLocalsOnly | SearchImportsOnly)));

/* If ident is "start at module scope", only look at module scope
*/
if (ident == Id.empty)
{
// Look for module scope
Expand All @@ -432,33 +469,70 @@ struct Scope
continue;
if (Dsymbol s = sc.scopesym.isModule())
{
//printf("\tfound %s.%s\n", s->parent ? s->parent->toChars() : "", s->toChars());
//printMsg("\tfound", s);
if (pscopesym)
*pscopesym = sc.scopesym;
return s;
}
}
return null;
}
for (Scope* sc = &this; sc; sc = sc.enclosing)

Dsymbol searchScopes(int flags)
{
assert(sc != sc.enclosing);
if (!sc.scopesym)
continue;
//printf("\tlooking in scopesym '%s', kind = '%s'\n", sc->scopesym->toChars(), sc->scopesym->kind());
if (Dsymbol s = sc.scopesym.search(loc, ident, flags))
for (Scope* sc = &this; sc; sc = sc.enclosing)
{
if (ident == Id.length && sc.scopesym.isArrayScopeSymbol() && sc.enclosing && sc.enclosing.search(loc, ident, null, flags))
assert(sc != sc.enclosing);
if (!sc.scopesym)
continue;
//printf("\tlooking in scopesym '%s', kind = '%s', flags = x%x\n", sc.scopesym.toChars(), sc.scopesym.kind(), flags);

if (sc.scopesym.isModule())
flags |= SearchUnqualifiedModule; // tell Module.search() that SearchLocalsOnly is to be obeyed

if (Dsymbol s = sc.scopesym.search(loc, ident, flags))
{
warning(s.loc, "array 'length' hides other 'length' name in outer scope");
if (!(flags & (SearchImportsOnly | IgnoreErrors)) &&
ident == Id.length && sc.scopesym.isArrayScopeSymbol() &&
sc.enclosing && sc.enclosing.search(loc, ident, null, flags))
{
warning(s.loc, "array 'length' hides other 'length' name in outer scope");
}
//printMsg("\tfound local", s);
if (pscopesym)
*pscopesym = sc.scopesym;
return s;
}
//printf("\tfound %s.%s, kind = '%s'\n", s->parent ? s->parent->toChars() : "", s->toChars(), s->kind());
if (pscopesym)
*pscopesym = sc.scopesym;
return s;
// Stop when we hit a module, but keep going if that is not just under the global scope
if (sc.scopesym.isModule() && !(sc.enclosing && !sc.enclosing.enclosing))
break;
}
return null;
}
return null;

if (global.params.bug10378)
return searchScopes(flags);

// First look in local scopes
Dsymbol s = searchScopes(flags | SearchLocalsOnly);

if (s)
{
version (LOGSEARCH)
printMsg("-Scope.search() found local", s);
return s;
}

s = searchScopes(flags | SearchImportsOnly); // look in imported modules

version (LOGSEARCH)
{
if (s)
printMsg("-Scope.search() found import", s);
else
printf("-Scope.search() not found\n");
}
return s;
}

extern (C++) Dsymbol search_correct(Identifier ident)
Expand Down
4 changes: 2 additions & 2 deletions src/dstruct.d
View file
Expand Up @@ -576,9 +576,9 @@ public:
}
}

override final Dsymbol search(Loc loc, Identifier ident, int flags = IgnoreNone)
override final Dsymbol search(Loc loc, Identifier ident, int flags = SearchLocalsOnly)
{
//printf("%s.StructDeclaration::search('%s')\n", toChars(), ident->toChars());
//printf("%s.StructDeclaration::search('%s', flags = x%x)\n", toChars(), ident.toChars(), flags);
if (_scope && !symtab)
semantic(_scope);

Expand Down

0 comments on commit e5be073

Please sign in to comment.