www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Local imports hide local symbols

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Sep 23 2014
next sibling parent reply "Meta" <jared771 gmail.com> writes:
On Tuesday, 23 September 2014 at 18:34:51 UTC, Andrei 
Alexandrescu wrote:

 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;
}
Sep 23 2014
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/23/14, 11:48 AM, Meta wrote:

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

 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
Sep 23 2014
prev sibling next sibling parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Tue, Sep 23, 2014 at 11:34:51AM -0700, Andrei Alexandrescu via Digitalmars-d
wrote:

 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

-- 
"Holy war is an oxymoron." -- Lazarus Long
Sep 23 2014
next sibling parent reply "Meta" <jared771 gmail.com> writes:
On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via 
Digitalmars-d wrote:

 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


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.
Sep 23 2014
parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Tue, Sep 23, 2014 at 06:56:56PM +0000, Meta via Digitalmars-d wrote:

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

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


 
 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

-- 
Живёшь только однажды.
Sep 23 2014
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/23/14, 12:01 PM, H. S. Teoh via Digitalmars-d wrote:

 On Tue, Sep 23, 2014 at 06:56:56PM +0000, Meta via Digitalmars-d wrote:

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

 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


 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
Sep 23 2014
parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Tue, Sep 23, 2014 at 12:06:26PM -0700, Andrei Alexandrescu via Digitalmars-d
wrote:

 On 9/23/14, 12:01 PM, H. S. Teoh via Digitalmars-d wrote:

On Tue, Sep 23, 2014 at 06:56:56PM +0000, Meta via Digitalmars-d wrote:

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

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.








[...]

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

-- 
He who laughs last thinks slowest.
Sep 23 2014
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 23 September 2014 at 19:18:08 UTC, H. S. Teoh via 
Digitalmars-d wrote:

 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


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?
Sep 23 2014
parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Tue, Sep 23, 2014 at 07:47:59PM +0000, monarch_dodra via Digitalmars-d wrote:

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

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


 
 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

-- 
What do you get if you drop a piano down a mineshaft? A flat minor.
Sep 23 2014
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Tuesday, 23 September 2014 at 20:10:35 UTC, H. S. Teoh via 
Digitalmars-d wrote:

 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...


 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.


 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.
Sep 23 2014
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 09/23/2014 10:19 PM, monarch_dodra wrote:

 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?
Sep 23 2014
prev sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Tuesday, 23 September 2014 at 19:03:28 UTC, H. S. Teoh via
Digitalmars-d wrote:

 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).
Sep 23 2014
prev sibling next sibling parent reply "Peter Alexander" <peter.alexander.au gmail.com> writes:
On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via 
Digitalmars-d wrote:

 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.
Sep 23 2014
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 09/23/2014 09:01 PM, Peter Alexander wrote:

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

 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
}
Sep 23 2014
prev sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Tuesday, 23 September 2014 at 18:52:13 UTC, H. S. Teoh via
Digitalmars-d wrote:

 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.
Sep 23 2014
prev sibling next sibling parent ketmar via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Tue, 23 Sep 2014 11:50:22 -0700
"H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> wrote:


 (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. ;-)
Sep 23 2014
prev sibling next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Andrei Alexandrescu:


 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
Sep 23 2014
prev sibling parent "Brian Schott" <briancschott gmail.com> writes:
On Tuesday, 23 September 2014 at 18:34:51 UTC, Andrei 
Alexandrescu wrote:

 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
Sep 23 2014