https://issues.dlang.org/show_bug.cgi?id=17630

Jack Stouffer <jack@jackstouffer.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jack@jackstouffer.com
           Hardware|x86_64                      |All
                 OS|Linux                       |All

--

https://issues.dlang.org/show_bug.cgi?id=17630

--- Comment #1 from Seb <greensunny12@gmail.com> ---


Without the selective import:

test17630a.ScopeDsymbol::search(ident='Erase', flags=x0)
object.ScopeDsymbol::search(ident='Erase', flags=x1)
test17630b.ScopeDsymbol::search(ident='Erase', flags=x1)
    found in locals = 'test17630b.Erase'
test17630a.ScopeDsymbol::search(ident='Erase', flags=x28)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x30)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x28)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x30)
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x8)
    found in locals = '__anonymous.Erase'



With the selective import in foo:

__anonymous.ScopeDsymbol::search(ident='Erase', flags=x8)
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x8)
fail17630.ScopeDsymbol::search(ident='Erase', flags=x28)
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x28)
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x10)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x1)
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x10)
fail17630.ScopeDsymbol::search(ident='Erase', flags=x30)
object.ScopeDsymbol::search(ident='Erase', flags=x1)
...
fail_compilation/fail17630.d(11): Error: undefined identifier Erase


With a selective import of another symbol in bar:


test17630b.ScopeDsymbol::search(ident='NoErase', flags=x0)
    found in locals = 'test17630b.NoErase'
test17630b.ScopeDsymbol::search(ident='NoErase', flags=x28)
    found in locals = 'test17630b.NoErase'
test17630a.ScopeDsymbol::search(ident='Erase', flags=x0)
object.ScopeDsymbol::search(ident='Erase', flags=x1)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x2)
object.ScopeDsymbol::search(ident='Erase', flags=x3)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x2)
object.ScopeDsymbol::search(ident='Erase', flags=x3)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x2)
object.ScopeDsymbol::search(ident='Erase', flags=x3)
test17630a.ScopeDsymbol::search(ident='Erase', flags=x2)
fail_compilation/fail17630.d(9): Error: module imports.test17630a import
'Erase' not found, did you mean alias 'NoErase'?


Full test case for the DMD test suite:

diff --git a/test/fail_compilation/fail17630.d
b/test/fail_compilation/fail17630.d
new file mode 100644
index 000000000..08fe72a36
--- /dev/null
+++ b/test/fail_compilation/fail17630.d
@@ -0,0 +1,12 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail17630.d(12): Deprecation: imports.test17630a.Erase is not
visible from module fail17630
+---
+*/
+void main()
+{
+    import imports.test17630a : Erase;
+    //import imports.test17630a; // A non-selective import correctly errors
+    assert(Erase == 2);
+}
diff --git a/test/fail_compilation/imports/test17630a.d
b/test/fail_compilation/imports/test17630a.d
new file mode 100644
index 000000000..68003b1f9
--- /dev/null
+++ b/test/fail_compilation/imports/test17630a.d
@@ -0,0 +1,4 @@
+module imports.test17630a;
+//import imports.test17630b; // works __falsely__ as public import
+//import imports.test17630b : Erase; // <- works __falsely__ as public import
+import imports.test17630b : NoErase; // <- works correctly as private import
diff --git a/test/fail_compilation/imports/test17630b.d
b/test/fail_compilation/imports/test17630b.d
new file mode 100644
index 000000000..c02f4f88a
--- /dev/null
+++ b/test/fail_compilation/imports/test17630b.d
@@ -0,0 +1,3 @@
+module imports.test17630b;
+int Erase = 2;
+int NoErase = 3;

--

https://issues.dlang.org/show_bug.cgi?id=17630

--- Comment #2 from Seb <greensunny12@gmail.com> ---
> Can you investigate this a bit more and add that info to the bug report, including that it's been around for a while?


The leaked symbols are [found in the local `symtab` table](https://github.com/dlang/dmd/blob/master/src/ddmd/dsymbol.d#L1306)

```
test17630b.ScopeDsymbol::search(ident='Erase', flags=x0)
    found in locals = 'test17630b.Erase'
test17630b.ScopeDsymbol::search(ident='Erase', flags=x28)
    found in locals = 'test17630b.Erase'
```

However, the next four lines in the log shouldn't happen:

```
test17630a.ScopeDsymbol::search(ident='Erase', flags=x0)
    found in locals = 'test17630a.Erase'
test17630a.ScopeDsymbol::search(ident='Erase', flags=x28)
    found in locals = 'test17630a.Erase'
__anonymous.ScopeDsymbol::search(ident='Erase', flags=x8)
    found in locals = '__anonymous.Erase'
```

Now, when I look at the symbol table for test17630b it's built correctly:

```
DsymbolTable::insert(this = 0x7fc6d7dcbbe0, 'object')
DsymbolTable::insert(this = 0x7fc6d7dcbbe0, 'Erase')
DsymbolTable::insert(this = 0x7fc6d7dcbbe0, 'NoErase')
```

In fact even the members only get added for test17630b:

```
Import.addMember(this=object, sd=test17630b, sc=0x7f11f8e71b00)
Dsymbol::addMember('object')
Dsymbol::addMember(this = 0x7f11f8e71c20, 'object' scopesym = 'test17630b')
Dsymbol::addMember(this = 0x7f11f8e71c20, 'object' sds = 0x7f11fa2c2db0,
sds.symtab = 0x7f11f8e71d30)
Dsymbol::addMember('Erase')
Dsymbol::addMember(this = 0x7f11fa2c3aa0, 'Erase' scopesym = 'test17630b')
Dsymbol::addMember(this = 0x7f11fa2c3aa0, 'Erase' sds = 0x7f11fa2c2db0,
sds.symtab = 0x7f11f8e71d30)
Dsymbol::addMember('NoErase')
Dsymbol::addMember(this = 0x7f11fa2c3c60, 'NoErase' scopesym = 'test17630b')
Dsymbol::addMember(this = 0x7f11fa2c3c60, 'NoErase' sds = 0x7f11fa2c2db0,
sds.symtab = 0x7f11f8e71d30)
```

and in the main test file `Erase` doesn't get inserted into the symbol table:

```
__anonymous.ScopeDsymbol::search(ident='_Dmain', flags=x8)
__entrypoint.ScopeDsymbol::search(ident='_Dmain', flags=x28)
    found in locals = '__entrypoint._Dmain'
Import::semantic('imports.test17630a.object') object
test17630a.ScopeDsymbol::importScope(object, 2)
Import::semantic('imports.test17630a.imports') test17630b
test17630a.ScopeDsymbol::importScope(test17630b, 2)
Import::semantic('imports.test17630b.object') object
test17630b.ScopeDsymbol::importScope(object, 2)
DsymbolTable::insert(this = 0x7f975b424780, 'fail17630')
DsymbolTable::insert(this = 0x7f975b424850, 'imports')
DsymbolTable::insert(this = 0x7f975b424920, 'imports')
DsymbolTable::insert(this = 0x7f975b4249f0, 'fail17630')
Import::semantic('__anonymous') test17630a
Import::load('__anonymous') 0x7f975c8294b0
```


> This seems like a fairly important import leak remaining, would be good to explore how large it is.

It's _pretty_ large. AFAICT all module-level imports, non-selective imports
leak their symbols.
However, even selective imports leak their selected symbols:

```d
import imports.test17630b; // works __falsely__ as public import
import imports.test17630b : Erase; // works __falsely__ as public import
```

Funnily even `private import X;` doesn't fix it.

--

https://issues.dlang.org/show_bug.cgi?id=17630

Jack Stouffer <jack@jackstouffer.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |accepts-invalid

--

https://issues.dlang.org/show_bug.cgi?id=17630

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code@dawg.eu
            Summary|DMD treats imports as       |selective imports find
                   |public imports when         |symbols in private imports
                   |selectively imported        |of other modules

--

https://issues.dlang.org/show_bug.cgi?id=17630

--- Comment #3 from Martin Nowak <code@dawg.eu> ---
I'm going to remove the visibility deprecation anyhow. It's planned to remove the current find & check visibility mechanism with an partially ordered visibility one. This will allow us to merge the visibility control for import statements with the one used for other symbols.

--

https://issues.dlang.org/show_bug.cgi?id=17630

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|nobody@puremagic.com        |code@dawg.eu

--

https://issues.dlang.org/show_bug.cgi?id=17630

--- Comment #4 from Seb <greensunny12@gmail.com> ---
There is/was an open PR to fix this by Kenji: https://github.com/dlang/dmd/pull/3416

--

https://issues.dlang.org/show_bug.cgi?id=17630

Seb <greensunny12@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rumbu@rumbu.ro

--- Comment #5 from Seb <greensunny12@gmail.com> ---
*** Issue 18243 has been marked as a duplicate of this issue. ***

--

https://issues.dlang.org/show_bug.cgi?id=17630

Timothee Cour <timothee.cour2@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |timothee.cour2@gmail.com

--- Comment #6 from Timothee Cour <timothee.cour2@gmail.com> ---
just to make sure this is the same bug right?

```
// fun.d:
module fun;
import std.stdio;
// main.d:
import fun:writeln; // should not compile
void main(){ writeln("ok"); }
```

`rdmd main.d` works but should not compile

--