From 9bcb80a1aff7ab3aab11b64115621220a2771753 Mon Sep 17 00:00:00 2001 From: RazvanN7 Date: Wed, 10 Jan 2018 12:22:19 +0200 Subject: [PATCH 1/5] Fix Issue 18219 - Private import inside struct leaks symbols when used as VarDeclaration types --- src/dmd/dsymbol.d | 4 ++-- src/dmd/mtype.d | 8 +++++--- src/dmd/typesem.d | 4 ++-- test/fail_compilation/fail18219.d | 19 +++++++++++++++++++ test/fail_compilation/fail347.d | 2 +- test/fail_compilation/imports/a18219.d | 6 ++++++ test/fail_compilation/imports/b18219.d | 15 +++++++++++++++ 7 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 test/fail_compilation/fail18219.d create mode 100644 test/fail_compilation/imports/a18219.d create mode 100644 test/fail_compilation/imports/b18219.d diff --git a/src/dmd/dsymbol.d b/src/dmd/dsymbol.d index f04529f56dfa..1b6b38628274 100644 --- a/src/dmd/dsymbol.d +++ b/src/dmd/dsymbol.d @@ -669,7 +669,7 @@ extern (C++) class Dsymbol : RootObject * 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 +685,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/mtype.d b/src/dmd/mtype.d index a81d463f8eee..8f65d1098949 100644 --- 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,7 +6593,7 @@ 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); if (sm) goto L2; } diff --git a/src/dmd/typesem.d b/src/dmd/typesem.d index a7c73b7b2150..34e4fc197e61 100644 --- 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 new file mode 100644 index 000000000000..b14fae9f9dd1 --- /dev/null +++ b/test/fail_compilation/fail18219.d @@ -0,0 +1,19 @@ +// EXTRA_SOURCES: imports/b18219.d +/* +TEST_OUTPUT: +--- +fail_compilation/fail18219.d(15): Error: no property 'Foobar' for type 'AST', did you mean 'b18219.Foobar'? +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 index df3146343f96..61718df59e28 100644 --- 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 new file mode 100644 index 000000000000..1a88493ef15e --- /dev/null +++ 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 new file mode 100644 index 000000000000..6008c8a1e215 --- /dev/null +++ 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() {} From 580c475cf8c2cc3932638918dfaafccdab15ad13 Mon Sep 17 00:00:00 2001 From: RazvanN7 Date: Wed, 10 Jan 2018 12:40:23 +0200 Subject: [PATCH 2/5] Add ddoc style section for searchX --- src/dmd/dsymbol.d | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/dmd/dsymbol.d b/src/dmd/dsymbol.d index 1b6b38628274..75eadff71dae 100644 --- a/src/dmd/dsymbol.d +++ b/src/dmd/dsymbol.d @@ -664,8 +664,15 @@ 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 */ From 039cdf1a552a75c0559a0fa2cea33dfde8b0872c Mon Sep 17 00:00:00 2001 From: RazvanN7 Date: Mon, 15 Jan 2018 12:12:33 +0200 Subject: [PATCH 3/5] Fix symbol lookup to find public imports nested by structs --- src/dmd/astcodegen.d | 52 ++++++++++++++++--------------- src/dmd/expressionsem.d | 2 +- src/dmd/mtype.d | 3 +- test/fail_compilation/fail18219.d | 2 +- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/dmd/astcodegen.d b/src/dmd/astcodegen.d index 3691a782e41f..eee94e7a681b 100644 --- a/src/dmd/astcodegen.d +++ b/src/dmd/astcodegen.d @@ -7,32 +7,33 @@ 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; alias UserAttributeDeclaration = dmd.attrib.UserAttributeDeclaration; @@ -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/expressionsem.d b/src/dmd/expressionsem.d index f1173b547866..f7dbfdea2315 100644 --- 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 index 8f65d1098949..9472cb41114b 100644 --- a/src/dmd/mtype.d +++ b/src/dmd/mtype.d @@ -7271,6 +7271,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) { @@ -7279,7 +7280,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/test/fail_compilation/fail18219.d b/test/fail_compilation/fail18219.d index b14fae9f9dd1..146eb9813b11 100644 --- a/test/fail_compilation/fail18219.d +++ b/test/fail_compilation/fail18219.d @@ -3,7 +3,7 @@ TEST_OUTPUT: --- fail_compilation/fail18219.d(15): Error: no property 'Foobar' for type 'AST', did you mean 'b18219.Foobar'? -fail_compilation/fail18219.d(16): Error: no property 'Bar' for type 'AST' +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'? --- From 6d25a08a0700762fd0a349037b1fe2f94a1f218d Mon Sep 17 00:00:00 2001 From: RazvanN7 Date: Fri, 19 Jan 2018 11:37:28 +0200 Subject: [PATCH 4/5] Transform error into a deprecation message --- src/dmd/astcodegen.d | 4 ++-- src/dmd/mtype.d | 13 +++++++++++-- test/fail_compilation/fail18219.d | 6 +++--- test/fail_compilation/fail347.d | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/dmd/astcodegen.d b/src/dmd/astcodegen.d index eee94e7a681b..1f07e993c665 100644 --- a/src/dmd/astcodegen.d +++ b/src/dmd/astcodegen.d @@ -33,7 +33,7 @@ struct ASTCodegen public import dmd.typesem; public import dmd.ctfeexpr; -/* + alias initializerToExpression = dmd.initsem.initializerToExpression; alias typeToExpression = dmd.typesem.typeToExpression; alias UserAttributeDeclaration = dmd.attrib.UserAttributeDeclaration; @@ -59,5 +59,5 @@ struct ASTCodegen alias isType = dmd.dtemplate.isType; alias isExpression = dmd.dtemplate.isExpression; alias isTuple = dmd.dtemplate.isTuple; -*/ + } diff --git a/src/dmd/mtype.d b/src/dmd/mtype.d index 9472cb41114b..8770f0dcbb90 100644 --- 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.toPrettyChars()); + 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()); } @@ -6593,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, IgnorePrivateImports); + 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; diff --git a/test/fail_compilation/fail18219.d b/test/fail_compilation/fail18219.d index 146eb9813b11..8159a346e696 100644 --- a/test/fail_compilation/fail18219.d +++ b/test/fail_compilation/fail18219.d @@ -2,10 +2,10 @@ /* TEST_OUTPUT: --- -fail_compilation/fail18219.d(15): Error: no property 'Foobar' for type 'AST', did you mean 'b18219.Foobar'? +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'? +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; diff --git a/test/fail_compilation/fail347.d b/test/fail_compilation/fail347.d index 61718df59e28..737ece3f7ddc 100644 --- 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 'fail347.S.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`? --- */ From 6decb34dbf3e3728c93389227d713710209a1de3 Mon Sep 17 00:00:00 2001 From: RazvanN7 Date: Fri, 19 Jan 2018 11:44:36 +0200 Subject: [PATCH 5/5] Add changelog entry --- changelog/fix18219.dd | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/fix18219.dd diff --git a/changelog/fix18219.dd b/changelog/fix18219.dd new file mode 100644 index 000000000000..b806823d4043 --- /dev/null +++ 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