Merge pull request #7668 from RazvanN7/Lookup

Fix Issue 18219 - Private import inside struct leaks symbols when used as VarDeclaration types
dlang · Jan 22, 2018 · 1018db0 · 1018db0
2 parents 93562d2 + 6decb34
commit 1018db0
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 37 deletions.
diff --git a/changelog/fix18219.dd b/changelog/fix18219.dd
@@ -0,0 +1,8 @@
+Fix Issue 18219 - Private import inside struct leaks symbols when used as VarDeclaration types
+
+When implementing a struct which has a local private import
+the imported symbols are leaked if present in a VarDeclaration statement. For more information and examples see : https://issues.dlang.org/show_bug.cgi?id=18219
+
+Symbols from the private import should not visible outside the
+struct scope. A deprecation is now issued when such cases are
+encountered
diff --git a/src/dmd/astcodegen.d b/src/dmd/astcodegen.d
@@ -7,31 +7,32 @@ module dmd.astcodegen;
 
 struct ASTCodegen
 {
-    import dmd.aggregate;
-    import dmd.aliasthis;
-    import dmd.arraytypes;
-    import dmd.attrib;
-    import dmd.cond;
-    import dmd.dclass;
-    import dmd.declaration;
-    import dmd.denum;
-    import dmd.dimport;
-    import dmd.dmodule;
-    import dmd.dstruct;
-    import dmd.dsymbol;
-    import dmd.dtemplate;
-    import dmd.dversion;
-    import dmd.expression;
-    import dmd.func;
-    import dmd.hdrgen;
-    import dmd.init;
-    import dmd.initsem;
-    import dmd.mtype;
-    import dmd.nspace;
-    import dmd.statement;
-    import dmd.staticassert;
-    import dmd.typesem;
-    import dmd.ctfeexpr;
+    public import dmd.aggregate;
+    public import dmd.aliasthis;
+    public import dmd.arraytypes;
+    public import dmd.attrib;
+    public import dmd.cond;
+    public import dmd.dclass;
+    public import dmd.declaration;
+    public import dmd.denum;
+    public import dmd.dimport;
+    public import dmd.dmodule;
+    public import dmd.dstruct;
+    public import dmd.dsymbol;
+    public import dmd.dtemplate;
+    public import dmd.dversion;
+    public import dmd.expression;
+    public import dmd.func;
+    public import dmd.hdrgen;
+    public import dmd.init;
+    public import dmd.initsem;
+    public import dmd.mtype;
+    public import dmd.nspace;
+    public import dmd.statement;
+    public import dmd.staticassert;
+    public import dmd.typesem;
+    public import dmd.ctfeexpr;
+
 
     alias initializerToExpression   = dmd.initsem.initializerToExpression;
     alias typeToExpression          = dmd.typesem.typeToExpression;
@@ -58,4 +59,5 @@ struct ASTCodegen
     alias isType                    = dmd.dtemplate.isType;
     alias isExpression              = dmd.dtemplate.isExpression;
     alias isTuple                   = dmd.dtemplate.isTuple;
+
 }
diff --git a/src/dmd/dsymbol.d b/src/dmd/dsymbol.d
@@ -664,12 +664,19 @@ extern (C++) class Dsymbol : RootObject
     }
 
     /***************************************
-     * Search for identifier id as a member of 'this'.
-     * id may be a template instance.
+     * Search for identifier id as a member of `this`.
+     * `id` may be a template instance.
+     *
+     * Params:
+     *  loc = location to print the error messages
+     *  sc = the scope where the symbol is located
+     *  id = the id of the symbol
+     *  flags = the search flags which can be `SearchLocalsOnly` or `IgnorePrivateImports`
+     *
      * Returns:
      *      symbol found, NULL if not
      */
-    final Dsymbol searchX(Loc loc, Scope* sc, RootObject id)
+    final Dsymbol searchX(Loc loc, Scope* sc, RootObject id, int flags)
     {
         //printf("Dsymbol::searchX(this=%p,%s, ident='%s')\n", this, toChars(), ident.toChars());
         Dsymbol s = toAlias();
@@ -685,7 +692,7 @@ extern (C++) class Dsymbol : RootObject
         switch (id.dyncast())
         {
         case DYNCAST.identifier:
-            sm = s.search(loc, cast(Identifier)id);
+            sm = s.search(loc, cast(Identifier)id, flags);
             break;
         case DYNCAST.dsymbol:
             {

diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d
@@ -9523,7 +9523,7 @@ Expression semanticX(DotIdExp exp, Scope* sc)
 // If flag == 1, stop "not a property" error and return NULL.
 Expression semanticY(DotIdExp exp, Scope* sc, int flag)
 {
-    //printf("DotIdExp::semanticY(this = %p, '%s')\n", this, toChars());
+    //printf("DotIdExp::semanticY(this = %p, '%s')\n", exp, exp.toChars());
 
     //{ static int z; fflush(stdout); if (++z == 10) *(char*)0=0; }
 

diff --git a/src/dmd/mtype.d b/src/dmd/mtype.d
@@ -2558,7 +2558,7 @@ extern (C++) abstract class Type : RootObject
             if (this != Type.terror)
             {
                 if (s)
-                    error(loc, "no property `%s` for type `%s`, did you mean `%s`?", ident.toChars(), toChars(), s.toChars());
+                    error(loc, "no property `%s` for type `%s`, did you mean `%s`?", ident.toChars(), toChars(), s.toPrettyChars());
                 else
                     error(loc, "no property `%s` for type `%s`", ident.toChars(), toChars());
             }
@@ -6545,7 +6545,9 @@ extern (C++) abstract class TypeQualified : Type
 
                 Type t = s.getType(); // type symbol, type alias, or type tuple?
                 uint errorsave = global.errors;
-                Dsymbol sm = s.searchX(loc, sc, id);
+                int flags = t is null ? SearchLocalsOnly : IgnorePrivateImports;
+
+                Dsymbol sm = s.searchX(loc, sc, id, flags);
                 if (sm && !(sc.flags & SCOPE.ignoresymbolvisibility) && !symbolIsVisible(sc, sm))
                 {
                     .deprecation(loc, "`%s` is not visible from module `%s`", sm.toPrettyChars(), sc._module.toChars());
@@ -6591,9 +6593,18 @@ extern (C++) abstract class TypeQualified : Type
                         sm = t.toDsymbol(sc);
                         if (sm && id.dyncast() == DYNCAST.identifier)
                         {
-                            sm = sm.search(loc, cast(Identifier)id);
+                            sm = sm.search(loc, cast(Identifier)id /*, IgnorePrivateImports*/);
+                            // Deprecated in 2018-01.
+                            // Change to error by deleting the deprecation line and uncommenting
+                            // the above parameter. The error will be issued in Type.getProperty.
+                            // The deprecation is highlighted here to avoid a redundant call to
+                            // ScopeDsymbol.search.
+                            // @@@DEPRECATED_2019-01@@@.
                             if (sm)
+                            {
+                                .deprecation(loc, "`%s` is not visible from module `%s`", sm.toPrettyChars(), sc._module.toChars());
                                 goto L2;
+                            }
                         }
                     L3:
                         Expression e;
@@ -7269,6 +7280,7 @@ extern (C++) final class TypeStruct : Type
         Dsymbol searchSym()
         {
             int flags = sc.flags & SCOPE.ignoresymbolvisibility ? IgnoreSymbolVisibility : 0;
+
             Dsymbol sold = void;
             if (global.params.bug10378 || global.params.check10378)
             {
@@ -7277,7 +7289,7 @@ extern (C++) final class TypeStruct : Type
                     return sold;
             }
 
-            auto s = sym.search(e.loc, ident, flags | SearchLocalsOnly);
+            auto s = sym.search(e.loc, ident, flags | IgnorePrivateImports);
             if (global.params.check10378)
             {
                 alias snew = s;

diff --git a/src/dmd/typesem.d b/src/dmd/typesem.d
@@ -1236,7 +1236,7 @@ private extern (C++) final class TypeSemanticVisitor : Visitor
 
     override void visit(TypeStruct mtype)
     {
-        //printf("TypeStruct::semantic('%s')\n", sym.toChars());
+        //printf("TypeStruct::semantic('%s')\n", mtype.toChars());
         if (mtype.deco)
         {
             if (sc && sc.cppmangle != CPPMANGLE.def)
@@ -1279,7 +1279,7 @@ private extern (C++) final class TypeSemanticVisitor : Visitor
 
     override void visit(TypeClass mtype)
     {
-        //printf("TypeClass::semantic(%s)\n", sym.toChars());
+        //printf("TypeClass::semantic(%s)\n", mtype.toChars());
         if (mtype.deco)
         {
             if (sc && sc.cppmangle != CPPMANGLE.def)

diff --git a/test/fail_compilation/fail18219.d b/test/fail_compilation/fail18219.d
@@ -0,0 +1,19 @@
+// EXTRA_SOURCES: imports/b18219.d
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail18219.d(15): Deprecation: `b18219.Foobar` is not visible from module `fail18219`
+fail_compilation/fail18219.d(16): Error: no property `Bar` for type `AST`
+fail_compilation/fail18219.d(17): Error: no property `fun` for type `AST`, did you mean `b18219.fun`?
+fail_compilation/fail18219.d(18): Error: no property `Foobar` for type `AST`, did you mean `b18219.Foobar`?
+---
+*/
+import imports.a18219;
+
+void main()
+{
+    AST.Foobar t;
+    AST.Bar l;
+    AST.fun();
+    AST.Foobar.smeth();
+}
diff --git a/test/fail_compilation/fail347.d b/test/fail_compilation/fail347.d
@@ -2,7 +2,7 @@
 TEST_OUTPUT:
 ---
 fail_compilation/fail347.d(21): Error: undefined identifier `bbr`, did you mean variable `bar`?
-fail_compilation/fail347.d(22): Error: no property `ofo` for type `S`, did you mean `foo`?
+fail_compilation/fail347.d(22): Error: no property `ofo` for type `S`, did you mean `fail347.S.foo`?
 fail_compilation/fail347.d(23): Error: undefined identifier `strlenx`, did you mean function `strlen`?
 ---
 */

diff --git a/test/fail_compilation/imports/a18219.d b/test/fail_compilation/imports/a18219.d
@@ -0,0 +1,6 @@
+module a18219;
+
+struct AST
+{
+    import b18219;
+}
diff --git a/test/fail_compilation/imports/b18219.d b/test/fail_compilation/imports/b18219.d
@@ -0,0 +1,15 @@
+module b18219;
+
+class Foobar
+{
+    int a;
+    this(int a)
+    {
+        this.a = a;
+    }
+    static int smeth()
+    {
+        return 1;
+    }
+}
+void fun() {}