Bug 17630 – selective imports find symbols in private imports of other modules

Status
RESOLVED
Resolution
FIXED
Severity
blocker
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2017-07-10T03:37:01Z
Last change time
2018-01-23T17:25:37Z
Keywords
accepts-invalid
Assigned to
Martin Nowak
Creator
Seb

Comments

Comment #0 by greensunny12 — 2017-07-10T03:37:01Z
foo.d: --- module foo; import c; //import c : NotErase; // <- breaks build of module `bar` --- bar.d: --- module bar; unittest { import foo : Erase; // A non-selective import correctly errors assert(Erase == 2); } --- c.d: --- module c; int Erase = 2; int NotErase = 2; ---
Comment #1 by greensunny12 — 2017-07-12T20:45:36Z
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;
Comment #2 by greensunny12 — 2017-07-12T21:59:51Z
> 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.
Comment #3 by code — 2017-10-01T12:45:31Z
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.
Comment #4 by greensunny12 — 2018-01-02T03:31:16Z
There is/was an open PR to fix this by Kenji: https://github.com/dlang/dmd/pull/3416
Comment #5 by greensunny12 — 2018-01-17T02:23:57Z
*** Issue 18243 has been marked as a duplicate of this issue. ***
Comment #6 by timothee.cour2 — 2018-01-19T01:54:14Z
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
Comment #7 by razvan.nitu1305 — 2018-01-22T14:09:40Z
(In reply to Timothee Cour from comment #6) > 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 Indeed it is the same bug. I will submit a PR shortly
Comment #8 by razvan.nitu1305 — 2018-01-22T14:22:47Z
Comment #9 by github-bugzilla — 2018-01-23T17:25:36Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/7d23df1a037545ffed3b8632f2d84fa062573b77 Fix Issue 17630 - selective imports find symbols in private imports of other modules https://github.com/dlang/dmd/commit/52d26744cf6d6241d514f47c02cc35152412adda Merge pull request #7760 from RazvanN7/Issue_17630 Fix Issue 17630 - selective imports find symbols in private imports o… merged-on-behalf-of: Andrei Alexandrescu <[email protected]>