On Mon, 19 Dec 2011 15:53:02 -0800, Jakob Ovrum <jakobovrum@gmail.com> wrote:

> On Monday, 19 December 2011 at 21:04:28 UTC, so wrote:
>> On Mon, 19 Dec 2011 22:24:18 +0200, Adam Wilson <flyboynw@gmail.com> wrote:
>>
>>> So the consensus is to keep all private members/imports/etc?
>>
>> Ideally to get most out of the auto generation of di files (by definition of module system) we need to strip ALL private stuff.
>> Only this way we can say the automation is on par with handcrafting.
>> But if it does not look doable somehow, we can always handcraft them (which i'll generally prefer),
>
> Private members can still be used from public templates and whatnot. You can't take them out unless you can statically proved all references within the same .di to that private member was removed in the stripping process.

It sounds at this point we will just be leaving everything private in the DI files, sans implementations.

> foo.d
> ------
>
> private i = 42;
>
> // Is a public template, has to be left in
> void bar()()
> {
>     /* Uses 'i' */
> }
>
> void baz()
> {
>     /* Also uses 'i' */
> }
> ------
>
> foo.di
> ------
>
> void bar()()
> {
>     /* Still uses 'i', where did it go!? */
> }
>
> void baz(); // Fine, no longer need 'i'
> ------
>
> Also, on a side note, function parameters can use types from private imports too. Be it function, data or import, it has to be left in even if private.

This was discovered by drey_ on IRC and will be fixed by leaving all privates intact. I'll upload a patch to my git account this evening.

-- 
Adam Wilson
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/

On Mon, 19 Dec 2011 08:53:21 -0800, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 12/19/11 2:11 AM, Adam Wilson wrote:
>> As you may all be aware, I've been trying to improve the automated
>> generation of .di files and I now have something that I feel is
>> testable. Currently the new code only makes the following changes.
>>
>> 1. Function Implementations are removed
>> 2. Private Function Declarations are removed.
>> 3. Variable Initializers, except for const, are removed.
>
> Don't forget immutable.

I did it, but it wasn't pretty. I had to pass the immutable state via the HeaderGenState struct.

>> Everything else is left alone. Templates and mixins are not addressed
>> with this code and *should* not be modified. That's where I need your
>> help, the test cases I have written cover some basic scenarios but I
>> don't have the capability to test these changes with the diverse code
>> base that the community has created.
>>
>> drey_ from IRC was kind enough to test build Derelict with the changes
>> and has discovered a potential issue around private imports. Derelict
>> uses private imports that contain types which are used in function alias
>> declarations. As one would expect, this caused many compiler errors.
>> Currently, I feel that private imports should be stripped from the DI
>> file as they are intended to be internal to the module. However, I want
>> to put it to the community to decide, and I would especially appreciate
>> Mr. Bright's opinion on private imports in DI files.
>
> I suspect you'd still need the private imports because template code may use them.

Privates are now all in.

> This is great work. It's almost a textbook example of how one can make a great positive impact on D's development by finding an area of improvement and working on it. Congratulations!
>
> You may want to generate DIs for Phobos and take a look at them. Phobos uses a vast array of D's capabilities so it's an effective unittest for DI generation.
>
>
> Thanks,
>
> Andrei


-- 
Adam Wilson
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/

On Mon, 19 Dec 2011 00:11:25 -0800, Adam Wilson <flyboynw@gmail.com> wrote:

The latest DI generation code is now on my Git account and ready for testing. It fixes the following issues:

1.	Privates should exist in the DI file to support public templates.
2.	Template classes and functions retain their implementations.
3.	Immutable types should retain their initializers.

At this point I could really use testing; you can download them from my git account here: https://LightBender@github.com/LightBender/dmd.git
I am trying to get myself setup for building phobos as a test but this is proving to be a lengthy process.

-- 
Adam Wilson
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/

Derelict works ok now, good work!

However, the .di files end up eating newlines.

Before:
double ALLEGRO_USECS_TO_SECS(long x)
{
return x / 1e+06;
}
double ALLEGRO_MSECS_TO_SECS(long x)
{
return x / 1000;
}
double ALLEGRO_BPS_TO_SECS(int x)
{
return 1 / x;
}

After:
double ALLEGRO_USECS_TO_SECS(long x);double ALLEGRO_MSECS_TO_SECS(long
x);double ALLEGRO_BPS_TO_SECS(int x);

I've tried merging https://github.com/D-Programming-Language/dmd/pull/538 but it doesn't fix this.

On 12/20/11 2:03 AM, Adam Wilson wrote:
> On Mon, 19 Dec 2011 00:11:25 -0800, Adam Wilson <flyboynw@gmail.com> wrote:
>
> The latest DI generation code is now on my Git account and ready for
> testing. It fixes the following issues:
>
> 1. Privates should exist in the DI file to support public templates.
> 2. Template classes and functions retain their implementations.
> 3. Immutable types should retain their initializers.

Great!

> At this point I could really use testing; you can download them from my
> git account here: https://LightBender@github.com/LightBender/dmd.git
> I am trying to get myself setup for building phobos as a test but this
> is proving to be a lengthy process.

Nah, it's much easier than you might think. The posix.mak makefile is very small for what it does, and you need to literally change one line of code to make it generate .di headers.


Andrei

On Tue, 20 Dec 2011 03:49:35 -0800, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:

> Derelict works ok now, good work!

This is fantastic news! Thanks for helping to test these changes. :-)

> However, the .di files end up eating newlines.
>
> Before:
> double ALLEGRO_USECS_TO_SECS(long x)
> {
> return x / 1e+06;
> }
> double ALLEGRO_MSECS_TO_SECS(long x)
> {
> return x / 1000;
> }
> double ALLEGRO_BPS_TO_SECS(int x)
> {
> return 1 / x;
> }
>
> After:
> double ALLEGRO_USECS_TO_SECS(long x);double ALLEGRO_MSECS_TO_SECS(long
> x);double ALLEGRO_BPS_TO_SECS(int x);
>
> I've tried merging
> https://github.com/D-Programming-Language/dmd/pull/538 but it doesn't
> fix this.

I noticed similar issues before I made changes to DI generation, but fixing it wasn't as high a priority as getting it working. For now I am going to continue testing the current changes, however, I would be open to prettifying it later. I think the simple fix is to add an extra newline between those functions.

-- 
Adam Wilson
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/

On Tue, 20 Dec 2011 04:41:11 -0800, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 12/20/11 2:03 AM, Adam Wilson wrote:
>> On Mon, 19 Dec 2011 00:11:25 -0800, Adam Wilson <flyboynw@gmail.com> wrote:
>>
>> The latest DI generation code is now on my Git account and ready for
>> testing. It fixes the following issues:
>>
>> 1. Privates should exist in the DI file to support public templates.
>> 2. Template classes and functions retain their implementations.
>> 3. Immutable types should retain their initializers.
>
> Great!
>
>> At this point I could really use testing; you can download them from my
>> git account here: https://LightBender@github.com/LightBender/dmd.git
>> I am trying to get myself setup for building phobos as a test but this
>> is proving to be a lengthy process.
>
> Nah, it's much easier than you might think. The posix.mak makefile is very small for what it does, and you need to literally change one line of code to make it generate .di headers.
>
>
> Andrei

Is it be add the proper -H options? I'll try it tomorrow, events pretty much overran my day today. If the new DI gen can build valid Phobos DI's I think it is time to consider opening a pull request and get feedback from the maintainers. Any disagreement with that?

-- 
Adam Wilson
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/

Am 19.12.2011 09:11, schrieb Adam Wilson:
> As you may all be aware, I've been trying to improve the automated
> generation of .di files and I now have something that I feel is
> testable. Currently the new code only makes the following changes.
>
> 1. Function Implementations are removed
> 2. Private Function Declarations are removed.
> 3. Variable Initializers, except for const, are removed.
>
> Everything else is left alone. Templates and mixins are not addressed
> with this code and *should* not be modified. That's where I need your
> help, the test cases I have written cover some basic scenarios but I
> don't have the capability to test these changes with the diverse code
> base that the community has created.
>
> drey_ from IRC was kind enough to test build Derelict with the changes
> and has discovered a potential issue around private imports. Derelict
> uses private imports that contain types which are used in function alias
> declarations. As one would expect, this caused many compiler errors.
> Currently, I feel that private imports should be stripped from the DI
> file as they are intended to be internal to the module. However, I want
> to put it to the community to decide, and I would especially appreciate
> Mr. Bright's opinion on private imports in DI files.
>
> If anyone wishes to test my changes against their code, you can download
> them from my git account here:
> https://LightBender@github.com/LightBender/dmd.git
> I only ask that you inform me of anything broken and provide a test case
> that reproduces the error.
>
> The following is my current test case, it comes from a module in my
> project that has been modified to include the various tests I and others
> have been able to think of. I know for a fact that it is missing a great
> number of features available in D, and I would really appreciate any
> tests you might wish to add to this case.
>
> module Horizon.Collections.Basic;
>
> export void TestFunc(int a) {}
> private void TestFunc2(int b) {}
>
> const gss = "__gshared";
>
> export interface IEnumerator
> {
> export @property Object Current();
>
> export bool MoveNext();
> export void Reset();
> }
>
> export interface ICollection
> {
> export @property uint Count();
>
> export int opApply(int delegate(ref Object) dg);
> export IEnumerator GetEnumerator();
> }
>
> export interface IList : ICollection
> {
> export @property bool IsFixedSize();
> export @property bool IsReadOnly();
>
> export void Add(Object Value);
> export void Clear();
> export bool Contains(Object Value);
> export Object opIndex(ulong Index);
> export void opIndexAssign(Object Value, ulong Index);
> export int IndexOf(Object Value);
> export void Insert(ulong Index, Object Value);
> export void Remove(Object Value);
> export void RemoveAt(ulong Index);
> }
>
> export struct StructTesting
> {
> private int i = 0;
> protected int j = 1;
>
> export void ExportedStructFunc() {}
> private void PrivateStructFunc() {}
> }
>
> export class List : IList
> {
> export this(uint Capacity)
> {
> vCapacity = Capacity;
> internal = new Object[4];
> }
>
> private Object[] internal;
>
> private uint vCapacity = 0;
> export @property uint Capacity()
> {
> return vCapacity;
> }
>
> private uint vCount = 0;
> export @property uint Count()
> {
> return vCount;
> }
>
> export void Add(Object Value)
> {
> if(vCount == vCapacity) internal.length *= 2;
>
> internal[++vCount] = Value;
> }
>
> export int opApply(int delegate(ref Object) dg) { return 0; }
> export IEnumerator GetEnumerator() {return null;}
>
> export @property bool IsFixedSize() {return false;}
> export @property bool IsReadOnly() {return false;}
>
> export void Clear() { return; }
> export bool Contains(Object Value) {return false;}
> export Object opIndex(ulong Index) {return null;}
> export void opIndexAssign(Object Value, ulong Index) {}
> export int IndexOf(Object Value) {return 0;}
> export void Insert(ulong Index, Object Value) {}
> export void Remove(Object Value) {}
> export void RemoveAt(ulong Index) {}
> private int FooTest(int c) { return c*2; }
> protected int ProtectedTest(int d) { return d*3; }
> }
>
>
>
> This is the DI file as exported with the generation changes:
>
> // D import file generated from 'Basic.d'
> module Horizon.Collections.Basic;
> export void TestFunc(int a);
> const gss = "__gshared";
> export interface IEnumerator
> {
> export @property Object Current();
>
> export bool MoveNext();
> export void Reset();
> }
>
> export interface ICollection
> {
> export @property uint Count();
>
> export int opApply(int delegate(ref Object) dg);
> export IEnumerator GetEnumerator();
> }
>
> export interface IList : ICollection
> {
> export @property bool IsFixedSize();
>
> export @property bool IsReadOnly();
>
> export void Add(Object Value);
> export void Clear();
> export bool Contains(Object Value);
> export Object opIndex(ulong Index);
> export void opIndexAssign(Object Value, ulong Index);
> export int IndexOf(Object Value);
> export void Insert(ulong Index, Object Value);
> export void Remove(Object Value);
> export void RemoveAt(ulong Index);
> }
>
> export struct StructTesting
> {
> private int i;
>
> protected int j;
>
> export void ExportedStructFunc();
> }
>
> export class List : IList
> {
> export this(uint Capacity);
> private Object[] internal;
>
> private uint vCapacity;
>
> export @property uint Capacity();
>
> private uint vCount;
>
> export @property uint Count();
>
> export void Add(Object Value);
> export int opApply(int delegate(ref Object) dg);
> export IEnumerator GetEnumerator();
> export @property bool IsFixedSize();
>
> export @property bool IsReadOnly();
>
> export void Clear();
> export bool Contains(Object Value);
> export Object opIndex(ulong Index);
> export void opIndexAssign(Object Value, ulong Index);
> export int IndexOf(Object Value);
> export void Insert(ulong Index, Object Value);
> export void Remove(Object Value);
> export void RemoveAt(ulong Index);
> protected int ProtectedTest(int d);
> }
>
> Let me know what you think!
>

Just out of curiousity, could removing private function delcerations lead to a different vtable layout? (And therefore crash the application)

-- 
Kind Regards
Benjamin Thaut

On 12/21/2011 04:58 PM, Benjamin Thaut wrote:
> Am 19.12.2011 09:11, schrieb Adam Wilson:
>> As you may all be aware, I've been trying to improve the automated
>> generation of .di files and I now have something that I feel is
>> testable. Currently the new code only makes the following changes.
>>
>> 1. Function Implementations are removed
>> 2. Private Function Declarations are removed.
>> 3. Variable Initializers, except for const, are removed.
>>
>> Everything else is left alone. Templates and mixins are not addressed
>> with this code and *should* not be modified. That's where I need your
>> help, the test cases I have written cover some basic scenarios but I
>> don't have the capability to test these changes with the diverse code
>> base that the community has created.
>>
>> drey_ from IRC was kind enough to test build Derelict with the changes
>> and has discovered a potential issue around private imports. Derelict
>> uses private imports that contain types which are used in function alias
>> declarations. As one would expect, this caused many compiler errors.
>> Currently, I feel that private imports should be stripped from the DI
>> file as they are intended to be internal to the module. However, I want
>> to put it to the community to decide, and I would especially appreciate
>> Mr. Bright's opinion on private imports in DI files.
>>
>> If anyone wishes to test my changes against their code, you can download
>> them from my git account here:
>> https://LightBender@github.com/LightBender/dmd.git
>> I only ask that you inform me of anything broken and provide a test case
>> that reproduces the error.
>>
>> The following is my current test case, it comes from a module in my
>> project that has been modified to include the various tests I and others
>> have been able to think of. I know for a fact that it is missing a great
>> number of features available in D, and I would really appreciate any
>> tests you might wish to add to this case.
>>
>> module Horizon.Collections.Basic;
>>
>> export void TestFunc(int a) {}
>> private void TestFunc2(int b) {}
>>
>> const gss = "__gshared";
>>
>> export interface IEnumerator
>> {
>> export @property Object Current();
>>
>> export bool MoveNext();
>> export void Reset();
>> }
>>
>> export interface ICollection
>> {
>> export @property uint Count();
>>
>> export int opApply(int delegate(ref Object) dg);
>> export IEnumerator GetEnumerator();
>> }
>>
>> export interface IList : ICollection
>> {
>> export @property bool IsFixedSize();
>> export @property bool IsReadOnly();
>>
>> export void Add(Object Value);
>> export void Clear();
>> export bool Contains(Object Value);
>> export Object opIndex(ulong Index);
>> export void opIndexAssign(Object Value, ulong Index);
>> export int IndexOf(Object Value);
>> export void Insert(ulong Index, Object Value);
>> export void Remove(Object Value);
>> export void RemoveAt(ulong Index);
>> }
>>
>> export struct StructTesting
>> {
>> private int i = 0;
>> protected int j = 1;
>>
>> export void ExportedStructFunc() {}
>> private void PrivateStructFunc() {}
>> }
>>
>> export class List : IList
>> {
>> export this(uint Capacity)
>> {
>> vCapacity = Capacity;
>> internal = new Object[4];
>> }
>>
>> private Object[] internal;
>>
>> private uint vCapacity = 0;
>> export @property uint Capacity()
>> {
>> return vCapacity;
>> }
>>
>> private uint vCount = 0;
>> export @property uint Count()
>> {
>> return vCount;
>> }
>>
>> export void Add(Object Value)
>> {
>> if(vCount == vCapacity) internal.length *= 2;
>>
>> internal[++vCount] = Value;
>> }
>>
>> export int opApply(int delegate(ref Object) dg) { return 0; }
>> export IEnumerator GetEnumerator() {return null;}
>>
>> export @property bool IsFixedSize() {return false;}
>> export @property bool IsReadOnly() {return false;}
>>
>> export void Clear() { return; }
>> export bool Contains(Object Value) {return false;}
>> export Object opIndex(ulong Index) {return null;}
>> export void opIndexAssign(Object Value, ulong Index) {}
>> export int IndexOf(Object Value) {return 0;}
>> export void Insert(ulong Index, Object Value) {}
>> export void Remove(Object Value) {}
>> export void RemoveAt(ulong Index) {}
>> private int FooTest(int c) { return c*2; }
>> protected int ProtectedTest(int d) { return d*3; }
>> }
>>
>>
>>
>> This is the DI file as exported with the generation changes:
>>
>> // D import file generated from 'Basic.d'
>> module Horizon.Collections.Basic;
>> export void TestFunc(int a);
>> const gss = "__gshared";
>> export interface IEnumerator
>> {
>> export @property Object Current();
>>
>> export bool MoveNext();
>> export void Reset();
>> }
>>
>> export interface ICollection
>> {
>> export @property uint Count();
>>
>> export int opApply(int delegate(ref Object) dg);
>> export IEnumerator GetEnumerator();
>> }
>>
>> export interface IList : ICollection
>> {
>> export @property bool IsFixedSize();
>>
>> export @property bool IsReadOnly();
>>
>> export void Add(Object Value);
>> export void Clear();
>> export bool Contains(Object Value);
>> export Object opIndex(ulong Index);
>> export void opIndexAssign(Object Value, ulong Index);
>> export int IndexOf(Object Value);
>> export void Insert(ulong Index, Object Value);
>> export void Remove(Object Value);
>> export void RemoveAt(ulong Index);
>> }
>>
>> export struct StructTesting
>> {
>> private int i;
>>
>> protected int j;
>>
>> export void ExportedStructFunc();
>> }
>>
>> export class List : IList
>> {
>> export this(uint Capacity);
>> private Object[] internal;
>>
>> private uint vCapacity;
>>
>> export @property uint Capacity();
>>
>> private uint vCount;
>>
>> export @property uint Count();
>>
>> export void Add(Object Value);
>> export int opApply(int delegate(ref Object) dg);
>> export IEnumerator GetEnumerator();
>> export @property bool IsFixedSize();
>>
>> export @property bool IsReadOnly();
>>
>> export void Clear();
>> export bool Contains(Object Value);
>> export Object opIndex(ulong Index);
>> export void opIndexAssign(Object Value, ulong Index);
>> export int IndexOf(Object Value);
>> export void Insert(ulong Index, Object Value);
>> export void Remove(Object Value);
>> export void RemoveAt(ulong Index);
>> protected int ProtectedTest(int d);
>> }
>>
>> Let me know what you think!
>>
>
> Just out of curiousity, could removing private function delcerations
> lead to a different vtable layout? (And therefore crash the application)
>

No, private implies final.

On 12/21/11 2:12 AM, Adam Wilson wrote:
> On Tue, 20 Dec 2011 04:41:11 -0800, Andrei Alexandrescu
> <SeeWebsiteForEmail@erdani.org> wrote:
>
>> On 12/20/11 2:03 AM, Adam Wilson wrote:
>>> On Mon, 19 Dec 2011 00:11:25 -0800, Adam Wilson <flyboynw@gmail.com>
>>> wrote:
>>>
>>> The latest DI generation code is now on my Git account and ready for
>>> testing. It fixes the following issues:
>>>
>>> 1. Privates should exist in the DI file to support public templates.
>>> 2. Template classes and functions retain their implementations.
>>> 3. Immutable types should retain their initializers.
>>
>> Great!
>>
>>> At this point I could really use testing; you can download them from my
>>> git account here: https://LightBender@github.com/LightBender/dmd.git
>>> I am trying to get myself setup for building phobos as a test but this
>>> is proving to be a lengthy process.
>>
>> Nah, it's much easier than you might think. The posix.mak makefile is
>> very small for what it does, and you need to literally change one line
>> of code to make it generate .di headers.
>>
>>
>> Andrei
>
> Is it be add the proper -H options? I'll try it tomorrow, events pretty
> much overran my day today. If the new DI gen can build valid Phobos DI's
> I think it is time to consider opening a pull request and get feedback
> from the maintainers. Any disagreement with that?

I don't think we'll use .di for phobos for the time being. I was suggesting you do so in order to look over the generated .di files and make sure they work.

Andrei