Local imports hide local symbols

[Andrei Alexandrescu via Digitalmars-d]

I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to critical.
Please chime in with ideas for a good solution. Thanks! -- Andrei

[Meta via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:34:51 UTC, Andrei

Post by Andrei Alexandrescu via Digitalmars-d
I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to
critical. Please chime in with ideas for a good solution.
Thanks! -- Andrei

What about requiring all local imports to be statically imported?

module a;

int i;


module b;

int test(int i)
{
//Error, local imports must be static
//import a;

//Okay
static import a;
}

[Andrei Alexandrescu via Digitalmars-d]

Post by Meta via Digitalmars-d

Post by Andrei Alexandrescu via Digitalmars-d
I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to
critical. Please chime in with ideas for a good solution. Thanks! --
Andrei

What about requiring all local imports to be statically imported?

It's in the issue comments. I think it would work (along with named
imports). It would break code but only a relatively small fraction of
new code. -- Andrei

[H. S. Teoh via Digitalmars-d]

Post by Andrei Alexandrescu via Digitalmars-d
I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to critical.
Please chime in with ideas for a good solution. Thanks! -- Andrei

I can think of a few:

1) Change lookup rules so that symbols pulled in by local import are
found last. Walter has stated that he disagrees with this approach
because it complicates symbol lookup rules.

2) Emit a compile error if any symbol pulled in by the local import
shadows a symbol currently in scope, according to the same rules as
declaring local variables that shadow identically-named variables in an
outer scope within the current function body.

3) What you proposed in the bugnotes: only allow `static import xyz;`
and `import xyz : a, b, c;` at local scope.

As far as breakage of existing code is concerned, (1) and (2) will only
break code where there was already a problem (an outer scope's symbol is
being shadowed, most likely unintentionally, by the import). (3) will
likely cause backlash because it will break a LOT of code that currently
compiles and likely to have no actual problems. Not to mention that
while naming specific symbols to import works for trivial cases, it can
quickly and easily devolve into inordinately long lists of symbols once
the local scope grows into non-trivial code. People are unlikely to be
happy with this.

Which leads to this variation of (2):

2b) Allow unqualified `import xyz;` in local scope, but only if NONE of
the imported symbols shadows ANY symbol visible from that scope.


T

[ketmar via Digitalmars-d]

On Tue, 23 Sep 2014 11:50:22 -0700

Post by H. S. Teoh via Digitalmars-d
(3) will likely cause backlash because it will break a LOT
of code that currently compiles and likely to have no actual
problems.

we always can make a deprecation warning first.

(dreaming) i want wildcard/regexp imports! both for module names and
for identifiers. something like `import std.*;` and
`import mymodule : pfx*;` at least. maybe i'll do another
universally-hated-patch(tm) for this. ;-)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/digitalmars-d/attachments/20140923/e14624cb/attachment.sig>

[Meta via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local
import are
found last. Walter has stated that he disagrees with this
approach
because it complicates symbol lookup rules.
2) Emit a compile error if any symbol pulled in by the local
import
shadows a symbol currently in scope, according to the same
rules as
declaring local variables that shadow identically-named
variables in an
outer scope within the current function body.
3) What you proposed in the bugnotes: only allow `static import
xyz;`
and `import xyz : a, b, c;` at local scope.
As far as breakage of existing code is concerned, (1) and (2)
will only
break code where there was already a problem (an outer scope's
symbol is
being shadowed, most likely unintentionally, by the import).
(3) will
likely cause backlash because it will break a LOT of code that
currently
compiles and likely to have no actual problems. Not to mention
that
while naming specific symbols to import works for trivial
cases, it can
quickly and easily devolve into inordinately long lists of
symbols once
the local scope grows into non-trivial code. People are
unlikely to be
happy with this.
2b) Allow unqualified `import xyz;` in local scope, but only if
NONE of
the imported symbols shadows ANY symbol visible from that scope.
T

The only tenable option from that list seems to be 1. 2 and 2b
either compile or not depending on the implementation details of
the module (i.e., add a symbol i to a module and it may break
code in an entirely different module that imports your module),
and 3 will break a lot of valid code.

[H. S. Teoh via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local import are
found last. Walter has stated that he disagrees with this approach
because it complicates symbol lookup rules.
2) Emit a compile error if any symbol pulled in by the local import
shadows a symbol currently in scope, according to the same rules as
declaring local variables that shadow identically-named variables in
an outer scope within the current function body.
3) What you proposed in the bugnotes: only allow `static import xyz;`
and `import xyz : a, b, c;` at local scope.
As far as breakage of existing code is concerned, (1) and (2) will
only break code where there was already a problem (an outer scope's
symbol is being shadowed, most likely unintentionally, by the
import). (3) will likely cause backlash because it will break a LOT
of code that currently compiles and likely to have no actual
problems. Not to mention that while naming specific symbols to import
works for trivial cases, it can quickly and easily devolve into
inordinately long lists of symbols once the local scope grows into
non-trivial code. People are unlikely to be happy with this.
2b) Allow unqualified `import xyz;` in local scope, but only if NONE
of the imported symbols shadows ANY symbol visible from that scope.
T

The only tenable option from that list seems to be 1. 2 and 2b either
compile or not depending on the implementation details of the module
(i.e., add a symbol i to a module and it may break code in an entirely
different module that imports your module), and 3 will break a lot of
valid code.

Good luck convincing Walter, then. :-( Or maybe if we can convince
Andrei to twist his arm hard enough... :-P


T

[Andrei Alexandrescu via Digitalmars-d]

Post by H. S. Teoh via Digitalmars-d

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local import are
found last. Walter has stated that he disagrees with this approach
because it complicates symbol lookup rules.
2) Emit a compile error if any symbol pulled in by the local import
shadows a symbol currently in scope, according to the same rules as
declaring local variables that shadow identically-named variables in
an outer scope within the current function body.
3) What you proposed in the bugnotes: only allow `static import xyz;`
and `import xyz : a, b, c;` at local scope.
As far as breakage of existing code is concerned, (1) and (2) will
only break code where there was already a problem (an outer scope's
symbol is being shadowed, most likely unintentionally, by the
import). (3) will likely cause backlash because it will break a LOT
of code that currently compiles and likely to have no actual
problems. Not to mention that while naming specific symbols to import
works for trivial cases, it can quickly and easily devolve into
inordinately long lists of symbols once the local scope grows into
non-trivial code. People are unlikely to be happy with this.
2b) Allow unqualified `import xyz;` in local scope, but only if NONE
of the imported symbols shadows ANY symbol visible from that scope.
T

The only tenable option from that list seems to be 1. 2 and 2b either
compile or not depending on the implementation details of the module
(i.e., add a symbol i to a module and it may break code in an entirely
different module that imports your module), and 3 will break a lot of
valid code.

Good luck convincing Walter, then. :-( Or maybe if we can convince
Andrei to twist his arm hard enough... :-P

This is a gaping hole that gets worse by the minute. We must fix it with
the next release. -- Andrei

[H. S. Teoh via Digitalmars-d]

Post by Andrei Alexandrescu via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local import
are found last. Walter has stated that he disagrees with this
approach because it complicates symbol lookup rules.
2) Emit a compile error if any symbol pulled in by the local import
shadows a symbol currently in scope, according to the same rules as
declaring local variables that shadow identically-named variables
in an outer scope within the current function body.
3) What you proposed in the bugnotes: only allow `static import
xyz;` and `import xyz : a, b, c;` at local scope.

[...]

Post by Andrei Alexandrescu via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d
2b) Allow unqualified `import xyz;` in local scope, but only if
NONE of the imported symbols shadows ANY symbol visible from that
scope.
T

The only tenable option from that list seems to be 1. 2 and 2b
either compile or not depending on the implementation details of the
module (i.e., add a symbol i to a module and it may break code in an
entirely different module that imports your module), and 3 will
break a lot of valid code.

Good luck convincing Walter, then. :-( Or maybe if we can convince
Andrei to twist his arm hard enough... :-P

This is a gaping hole that gets worse by the minute. We must fix it
with the next release. -- Andrei

Here's another idea:

4) Allow unqualified imports at local scope (even if imported symbols
will shadow currently symbols in scope), but emit a compile error if
such ambiguous symbols are referenced. So, this would be allowed:

----mod.d----
module mod;
string w, x, y;

----main.d----
void main() {
int x, y, z;
import mod;

w ~= "a";
z++;
}

But this would cause a compile error:

----mod.d----
module mod;
string x, y;

----main.d----
void main() {
int x, y, z;
import mod;

x++; // Error: ambiguous symbol 'x', could be local
// variable 'x' or mod.x
}


T

[monarch_dodra via Digitalmars-d]

On Tuesday, 23 September 2014 at 19:18:08 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
----mod.d----
module mod;
string x, y;
----main.d----
void main() {
int x, y, z;
import mod;
x++; // Error: ambiguous symbol 'x', could be local
// variable 'x' or mod.x
}
T

How do you disambiguate to say "the x I want is the local one" ?

IMO, simply make it that local imports work like global ones, but
scoped. Global imports don't have this issue, why should local
imports have special rules?

[H. S. Teoh via Digitalmars-d]

On Tuesday, 23 September 2014 at 19:18:08 UTC, H. S. Teoh via Digitalmars-d

Post by H. S. Teoh via Digitalmars-d
----mod.d----
module mod;
string x, y;
----main.d----
void main() {
int x, y, z;
import mod;
x++; // Error: ambiguous symbol 'x', could be local
// variable 'x' or mod.x
}
T

How do you disambiguate to say "the x I want is the local one" ?
IMO, simply make it that local imports work like global ones, but
scoped. Global imports don't have this issue, why should local
imports have special rules?

Sounds reasonable. How would that be implemented, though? Currently, in
the compiler, lookup is implemented via a linked list of Scope objects
that contain, among other things, a symbol table for the symbols
declared in that scope. A local import achieves locality by adding
symbols to the current (i.e., innermost) Scope, since doing otherwise
would cause those symbols to "spill" into the outer scopes and they will
persist past the lifetime of the current scope.

OTOH, it's this importing into the innermost scope that causes this
issue to begin with, since by definition, the innermost scope takes
precedence over outer scopes, so the imported symbols would shadow
symbols declared in outer scopes.

Implementing what you suggest would either involve treating imported
symbols separately (by having multiple parents per scope, which quickly
devolves into a mess, or otherwise having sibling pointers to imported
scopes, which also greatly complicates lookup logic), or sticking
symbols into outer scopes and keeping track of which symbols were
imported where so that they can be removed after we leave the current
scope -- which is fragile and would again add tons of complications to
the compiler.


T

[monarch_dodra via Digitalmars-d]

On Tuesday, 23 September 2014 at 20:10:35 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
Sounds reasonable. How would that be implemented, though?
Currently, in
the compiler, lookup is implemented via a linked list of Scope
objects
that contain, among other things, a symbol table for the symbols
declared in that scope. A local import achieves locality by
adding
symbols to the current (i.e., innermost) Scope, since doing
otherwise
would cause those symbols to "spill" into the outer scopes and
they will
persist past the lifetime of the current scope.

Arguably, that's not my problem...

Post by H. S. Teoh via Digitalmars-d
OTOH, it's this importing into the innermost scope that causes
this
issue to begin with, since by definition, the innermost scope
takes
precedence over outer scopes, so the imported symbols would
shadow
symbols declared in outer scopes.

I think that's the issue here. Are we actually importing "into"
the innermost scope, while shadowing any previous imports?

AFAIK, that's a behavior which is reserved for selective imports.

As I said, local imports, IMO, should behave in all aspects as a
global import. It simply only exists during its scope, but is not
actually any more "internal" than the rest. If a local import
creates a symbol ambiguity, then it's ambiguous, and compilation
ceases. I think that's the behavior we should be going for.

Post by H. S. Teoh via Digitalmars-d
Implementing what you suggest would either involve treating
imported
symbols separately (by having multiple parents per scope, which
quickly
devolves into a mess, or otherwise having sibling pointers to
imported
scopes, which also greatly complicates lookup logic), or
sticking
symbols into outer scopes and keeping track of which symbols
were
imported where so that they can be removed after we leave the
current
scope -- which is fragile and would again add tons of
complications to
the compiler.
T

Unfortunately, I don't know how the compiler works.

[Timon Gehr via Digitalmars-d]

As I said, local imports, IMO, should behave in all aspects as a global
import. It simply only exists during its scope, but is not actually any
more "internal" than the rest. If a local import creates a symbol
ambiguity, then it's ambiguous, and compilation ceases. I think that's
the behavior we should be going for.

I have previously suggested to first look up symbols in local scopes,
and only if no match is found within the current module, all imports
that are in visible scopes are considered. I think this has the effect
you are after. Is this what you are proposing?

[Timon Gehr via Digitalmars-d]

As I said, local imports, IMO, should behave in all aspects as a global
import. It simply only exists during its scope, but is not actually any
more "internal" than the rest. If a local import creates a symbol
ambiguity, then it's ambiguous, and compilation ceases. I think that's
the behavior we should be going for.

I have previously suggested to first look up symbols in local scopes,
and only if no match is found within the current module, all imports
that are in visible scopes are considered. I think this has the effect
you are after. Is this what you are proposing?

[deadalnix via Digitalmars-d]

On Tuesday, 23 September 2014 at 19:03:28 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
Good luck convincing Walter, then. :-( Or maybe if we can
convince
Andrei to twist his arm hard enough... :-P

In SDC? doing this represent ~30 lines on a 600+ line identifier
resolution (plus several other hundred line disambiguation for
overload and/or templates, constructor, IFTI and so on, that
aren't handled there).

[Peter Alexander via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local
import are
found last. Walter has stated that he disagrees with this
approach
because it complicates symbol lookup rules.

This.

[Timon Gehr via Digitalmars-d]

Post by Meta via Digitalmars-d
On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local import are
found last. Walter has stated that he disagrees with this approach
because it complicates symbol lookup rules.

This.

Agreed, but only if this is implemented such that more deeply nested
imports do not shadow less deeply nested imports.

I.e. if modules 'a' and 'b' both declare variables 'foo', then:

import a;

int fun(int foo){
import b; // fine
return foo; // fine, refers to parameter
}

auto gun(){
import b;
return foo; // error, could be a.foo or b.foo
}

[bearophile via Digitalmars-d]

Post by Andrei Alexandrescu via Digitalmars-d
I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to
critical. Please chime in with ideas for a good solution.
Thanks! -- Andrei

I think this code:

import foo;
void main() {}

should import only the "foo" identifier in the current
module/scope (or fail to do so). This requires you to use foo.bar
to access the name bar inside foo (and all the names inside the
module are private on default, so you need to add a "public" to
the ones you want to be visible outside the module). Plus if you
want to avoid specifying the module/package in your module you
can also use:

import foo: bar;

You can also do this, but this is discouraged:

import foo: *;

This last command is refused if some name shadowing happens.

When the module name clashes with a local name you have to use a
renaming import:

void main(string[] args) {
import margs = args;
}

But this design can't be used now in D. So I suggest to add anti
hijacking logic similar to the with() command. When such name
hiding errors are generated by a local import, the programmer has
to list imported names, or use renamed imports.

Bye,
bearophile

[deadalnix via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via

Post by H. S. Teoh via Digitalmars-d
1) Change lookup rules so that symbols pulled in by local
import are
found last. Walter has stated that he disagrees with this
approach
because it complicates symbol lookup rules.

That is dead simple compared to alias this, opDispatch or with
statement. That is a non argument.

This solution is simple (ie look local symbols, then look imports
is easy to understand) and would prevent hijacking and introduce
much less complications than alternatives.

[Brian Schott via Digitalmars-d]

On Tuesday, 23 September 2014 at 18:34:51 UTC, Andrei

Post by Andrei Alexandrescu via Digitalmars-d
I've raised https://issues.dlang.org/show_bug.cgi?id=10378 to
critical. Please chime in with ideas for a good solution.
Thanks! -- Andrei

In the meantime, anybody who wants to know if their code is
vulnerable to this problem can grab D-Scanner from git master and
run the style checker.

https://github.com/Hackerpilot/Dscanner/commit/95c8b1b19a1ec235aa50b7e9c2e0f4d5a4b1d404