Fix 13096 - Imported private identifiers conflict with public ones #3743
Conversation
| @@ -913,6 +913,9 @@ Dsymbol *ScopeDsymbol::search(Loc loc, Identifier *ident, int flags) | |||
| /* Don't find private members if ss is a module | |||
| */ | |||
| Dsymbol *s2 = ss->search(loc, ident, ss->isModule() ? IgnorePrivateMembers : IgnoreNone); | |||
| if (s && s2 && ss->isModule() && s2->prot() == PROTprivate) | |||
There was a problem hiding this comment.
This is not sufficient whtn s2 is function, template, or overloaded alias.
|
@9rnsr, added your use-cases to tests. Except for overloaded alias, which I can't quite understand what you mean by. The tests seem to pass. Did I miss anything? |
|
I meant that void foo1() {}
void foo2(int) {}
private alias foo = foo1;
public alias foo = foo2;
// in here, foo is an overloaded aliasAnd this fix would break following case: // test13096_private.d
private void foo() {}
public void foo(int) {}// test13096_public.d
void foo(string) {}// test13096.d
import imports.test13096_public;
import imports.test13096_private;
void main()
{
static assert(!__traits(compiles, foo()));
foo(1);
foo("str");
} |
|
@9rnsr, you're totally right. I've updated the code to not mess with overloadable symbols. Overloadables are still to be fixed, but it seems like it should be done elsewhere (e.g. resolveFuncCall(), etc). |
|
Issue 13096 seems to be a duplicate of issue 1238. |
ghost
changed the title
|
The single most annoying bug. I'm really impressed we have gone this far without fixing it. |
|
Yeah, when is this PR gonna get merged? It's just asking for an embarrassing failure in enterprise code when somebody accidentally adds private symbols to one module and causes breakage in other, unrelated modules. |
|
Seconding this, I've got a somewhat complicated module hierarchy and this On Oct 31, 2014 5:09 PM, "H. S. Teoh" notifications@github.com wrote:
|
|
The fix looks simple enough that it's worth merging. The commits will have to be squashed to one commit though. @9rnsr any objections for this pull? |
|
It's just two commits, is a squash really necessary? In any case, it's high time we merged this (barring objection by Walter & co), this is one of the embarrassing gaps in D's module system that greatly detracts from it. |
Well it's 3 commits, and AFAICT one commit adds code which the next commit removes (so they cancel each other out, and they're just noise at that point). |
|
Right, I missed the third one. Ignore what I just said. :-P |
|
Rebased and squashed. |
|
LGTM |
|
Pinging @9rnsr again since he knows more about this. |
|
The fact that this bug has been around since 2007 is crazy. This needs merging post-haste. |
|
Please add the test case that I've shown. |
| else if (ss->isModule() && s2->prot() == PROTprivate) | ||
| { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This is not bad, but does cover all cases?
ssis an overloadable public symbol, and
a.s2is an overloadable public symbol.
b.s2is an overloadable private symbol.
c.s2is an non-overloadable public symbol.
d.s2is an non-overloadable private symbol.ssis an overloadable private symbol, and
a.s2is an overloadable public symbol.
b.s2is an overloadable private symbol.
c.s2is an non-overloadable public symbol.
d.s2is an non-overloadable private symbol.ssis an non-overloadable public symbol, and
a.s2is an overloadable public symbol.
b.s2is an overloadable private symbol.
c.s2is an non-overloadable public symbol.
d.s2is an non-overloadable private symbol.ssis an non-overloadable priate symbol, and
a.s2is an overloadable public symbol.
b.s2is an overloadable private symbol.
c.s2is an non-overloadable public symbol.
d.s2is an non-overloadable private symbol.
|
This issue have been fixed. I think we can safely close this. |
core: Remove declarations with expired deprecations Signed-off-by: Nicholas Wilson <thewilsonator@users.noreply.github.com> Merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
Fix 13096