Thread overview
[dmd-internals] I think it's time to fix bug 314
Aug 12, 2010
Don Clugston
Aug 12, 2010
Leandro Lucarella
Aug 12, 2010
Don Clugston
Aug 12, 2010
Walter Bright
Aug 12, 2010
Don Clugston
Aug 12, 2010
Walter Bright
Aug 12, 2010
Leandro Lucarella
August 12, 2010
Bug 314 [module] Static, renamed, and selective imports are always public

It is a serious hole in the module system. It has had a patch for two years;
it has twice as many votes as anything else in Bugzilla. It's also had
two years of testing in LDC.
I think it's time to kill it.

I've reviewed the patch, and apart from a minor issue which I've fixed
(it couldn't compile object_.d), it seems to be good.
Applying it showed up bugs in druntime, and Phobos. This is a good
sign. I've checked in fixes for those bugs.
There's also a bug in the test suite: test24 should import std.string.

-----
To increase the chance of action on this, I repeat the patch here.
Because there are changes to a several places in import.c, I've attached it.
There are only really a couple of lines which change. Same file applies to D1.
The changes to the other two files are listed below.

Add these members to struct Import {} in import.h:

    enum PROT protection;
    enum PROT prot();

Add these three lines to dsymbol.c, line 757.

Dsymbol *ScopeDsymbol::search(Loc loc, Identifier *ident, int flags)
{
    //printf("%s->ScopeDsymbol::search(ident='%s', flags=x%x)\n",
toChars(), ident->toChars(), flags);
    //if (strcmp(ident->toChars(),"c") == 0) *(char*)0=0;

    // Look in symbols declared in this module
    Dsymbol *s = symtab ? symtab->lookup(ident) : NULL;
    //printf("\ts = %p, imports = %p, %d\n", s, imports, imports ?
imports->dim : 0);

+    // hide private nonlocal symbols
+    if (flags & 1 && s && s->prot() == PROTprivate)
+        s = NULL;

    if (s)
    {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: import.c
Type: application/octet-stream
Size: 10153 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20100812/c85881f3/attachment.obj>
August 12, 2010
Don Clugston, el 12 de agosto a las 15:18 me escribiste:
> Bug 314 [module] Static, renamed, and selective imports are always public
> 
> It is a serious hole in the module system. It has had a patch for two years;
> it has twice as many votes as anything else in Bugzilla. It's also had
> two years of testing in LDC.
> I think it's time to kill it.

You'll make a lot of people angry, now we'll have to and find a new favourite bug!


-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Fitter, happier, more productive, comfortable, not drinking too much,
regular exercise at the gym (3 days a week),
getting on better with your associate employee contemporaries,
August 12, 2010
After further comments from Christian Kamm, it seems the correct patch has never existed in Bugzilla. So the question is: Walter, are you willing to accept a patch as described in comment 9 of that bug? Or is there some other problem?


On 12 August 2010 16:22, Leandro Lucarella <luca at llucax.com.ar> wrote:
> Don Clugston, el 12 de agosto a las 15:18 me escribiste:
>> Bug 314 [module] Static, renamed, and selective imports are always public
>>
>> It is a serious hole in the module system. It has had a patch for two years;
>> it has twice as many votes as anything else in Bugzilla. It's also had
>> two years of testing in LDC.
>> I think it's time to kill it.
>
> You'll make a lot of people angry, now we'll have to and find a new favourite bug!
>
>
> --
> Leandro Lucarella (AKA luca) ? ? ? ? ? ? ? ? ? ? http://llucax.com.ar/
> ----------------------------------------------------------------------
> GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145 ?104C 949E BFB6 5F5A 8D05)
> ----------------------------------------------------------------------
> Fitter, happier, more productive, comfortable, not drinking too much,
> regular exercise at the gym (3 days a week),
> getting on better with your associate employee contemporaries,
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
August 12, 2010
I haven't looked at that patch, just that the original one only papered over the problem without solving it.

Don Clugston wrote:
> After further comments from Christian Kamm, it seems the correct patch has never existed in Bugzilla. So the question is: Walter, are you willing to accept a patch as described in comment 9 of that bug? Or is there some other problem?
>
>
> On 12 August 2010 16:22, Leandro Lucarella <luca at llucax.com.ar> wrote:
> 
>> Don Clugston, el 12 de agosto a las 15:18 me escribiste:
>> 
>>> Bug 314 [module] Static, renamed, and selective imports are always public
>>>
>>> It is a serious hole in the module system. It has had a patch for two years;
>>> it has twice as many votes as anything else in Bugzilla. It's also had
>>> two years of testing in LDC.
>>> I think it's time to kill it.
>>> 
>> You'll make a lot of people angry, now we'll have to and find a new favourite bug!
>>
>>
>> --
>> Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
>> ----------------------------------------------------------------------
>> GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
>> ----------------------------------------------------------------------
>> Fitter, happier, more productive, comfortable, not drinking too much,
>> regular exercise at the gym (3 days a week),
>> getting on better with your associate employee contemporaries,
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>>
>> 
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
> 
August 12, 2010
On 12 August 2010 21:42, Walter Bright <walter at digitalmars.com> wrote:
> I haven't looked at that patch, just that the original one only papered over the problem without solving it.

This was his comment:
-----------
I've updated the patch. Treating overloads correctly complicated the issue quite a bit. What I've done is to store the import protection in the AliasDeclarations and FuncAliasDeclarations generated by selective and renamed imports. These are then ignored when traversing the overload tree if they are in a different module than the one initiating the traversal.

I've also made the hiding of private symbols in ScopeDsymbol::search specific to AliasDeclarations generated by ImportStatement.
----
So you end up with a module and a protection level stored in each
alias declaration.
Is that approach reasonable?
August 12, 2010
I don't know. I have to look into it.

Don Clugston wrote:
> On 12 August 2010 21:42, Walter Bright <walter at digitalmars.com> wrote:
> 
>> I haven't looked at that patch, just that the original one only papered over
>> the problem without solving it.
>> 
>
> This was his comment:
> -----------
> I've updated the patch. Treating overloads correctly complicated the issue quite a bit. What I've done is to store the import protection in the AliasDeclarations and FuncAliasDeclarations generated by selective and renamed imports. These are then ignored when traversing the overload tree if they are in a different module than the one initiating the traversal.
>
> I've also made the hiding of private symbols in ScopeDsymbol::search specific to AliasDeclarations generated by ImportStatement.
> ----
> So you end up with a module and a protection level stored in each
> alias declaration.
> Is that approach reasonable?
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
> 
August 12, 2010
Walter Bright, el 12 de agosto a las 12:42 me escribiste:
> I haven't looked at that patch, just that the original one only papered over the problem without solving it.

Can you take a look at the patches mentioned in that comment?

	The LDC changesets are:
	http://www.dsource.org/projects/ldc/changeset/1358
	http://www.dsource.org/projects/ldc/changeset/1362
	I can make a patch against DMD if requested.

They are not big and all you have to do if you like them is let Christian Kamm know, sit and wait for the DMD :)

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Pitzulino! Pitzulino!
Todos a cantar por el tubo!
Pitzulino! Pitzulino!
Todos a cantar por el codo!