Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog/fix18219.dd
Original file line number Diff line number Diff line change
@@ -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
52 changes: 27 additions & 25 deletions src/dmd/astcodegen.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -58,4 +59,5 @@ struct ASTCodegen
alias isType = dmd.dtemplate.isType;
alias isExpression = dmd.dtemplate.isExpression;
alias isTuple = dmd.dtemplate.isTuple;

}
15 changes: 11 additions & 4 deletions src/dmd/dsymbol.d
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Params DDoc comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally DAutotest should check that and fail respectively (except that we broke it today - #7637)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok?

IMO, yes. Though I'm looking forward to replacing flags with a proper enum. But that's another PR.

{
//printf("Dsymbol::searchX(this=%p,%s, ident='%s')\n", this, toChars(), ident.toChars());
Dsymbol s = toAlias();
Expand All @@ -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:
{
Expand Down
2 changes: 1 addition & 1 deletion src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
20 changes: 16 additions & 4 deletions src/dmd/mtype.d
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/dmd/typesem.d
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions test/fail_compilation/fail18219.d
Original file line number Diff line number Diff line change
@@ -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();
}
2 changes: 1 addition & 1 deletion test/fail_compilation/fail347.d
Original file line number Diff line number Diff line change
Expand Up @@ -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`?
---
*/
Expand Down
6 changes: 6 additions & 0 deletions test/fail_compilation/imports/a18219.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module a18219;

struct AST
{
import b18219;
}
15 changes: 15 additions & 0 deletions test/fail_compilation/imports/b18219.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module b18219;

class Foobar
{
int a;
this(int a)
{
this.a = a;
}
static int smeth()
{
return 1;
}
}
void fun() {}