In reference to this link:
http://forum.dlang.org/post/50476C77.20608@googlemail.com


I'm currently working on dealing with each of these issues in the following pull (with the intention to merge back upstream where required).
https://github.com/D-Programming-GDC/GDC/pull/62

In order:

1. ClassInfo

The initialiser emitted will have two symbols, one public symbol with the TypeInfo_Class members, and a second private generated symbol for the interfaces array.  I can't forsee any way this could break compatibility with any existing compilied gdc (or perhaps even dmd/ldc) binaries out there.


2. TypeInfoStruct

Likewise to the above.


3. ModuleInfo

We can get the correct type as is defined in object.di through Module::moduleinfo, this would mean that all generated moduleinfo symbols will be the same size (rather than be of a variable size) padded out with zeros at the end.  However, this requires a front-end patch to store it as there is an implementation conflict because of MODULEINFO_IS_STRUCT macro, the type is a StructDeclaration (Module::moduleinfo is a ClassDeclaration type).

Going one step further, the type itself could probably be put up for a clean up.  Removing the struct New/Old implementation (keeping the 'New' for getting data members) and perhaps replace it something like the following.

struct ModuleInfo
{
  uint flags;           // Module flags
  uint index;           // Index into moduleinfo array
  void*[14] reserved;   // Padding large enough to contain
                        // all optional data added depending on flags.
}

This however is not really required, but am just throughing it out there as a thought.


Any thoughts on this?  (Looking at you Johannes).

Regards
Iain

Am Tue, 23 Apr 2013 17:10:43 +0200
schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:

> In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
> 
> 
> I'm currently working on dealing with each of these issues in the
> following pull (with the intention to merge back upstream where
> required).
> https://github.com/D-Programming-GDC/GDC/pull/62
> 
I'll make sure to have a look at this, but as always too little time...

> In order:
> 
> 1. ClassInfo
> 
> The initialiser emitted will have two symbols, one public symbol with the TypeInfo_Class members, and a second private generated symbol for the interfaces array.  I can't forsee any way this could break compatibility with any existing compilied gdc (or perhaps even dmd/ldc) binaries out there.

Sounds good. That should also be good for debug info.
> 
> 
> 2. TypeInfoStruct
> 
> Likewise to the above.

The situation here is a little different AFAIR. TypeInfoStruct has no interfaces array or something similar, it only had a variable size because the name data was part of the TypeInfoStruct type. Now name is just stored as a slice and the data is stored in a custom symbol. So the TypeInfoStruct size is fixed and there's no need to do anything special about it.

https://github.com/D-Programming-Language/dmd/pull/1128/files

> 
> 
> 3. ModuleInfo
> 
> We can get the correct type as is defined in object.di through Module::moduleinfo, this would mean that all generated moduleinfo symbols will be the same size (rather than be of a variable size) padded out with zeros at the end.  However, this requires a front-end patch to store it as there is an implementation conflict because of MODULEINFO_IS_STRUCT macro, the type is a StructDeclaration (Module::moduleinfo is a ClassDeclaration type).
> 
> Going one step further, the type itself could probably be put up for a clean up.  Removing the struct New/Old implementation (keeping the 'New' for getting data members) and perhaps replace it something like the following.
> 
> struct ModuleInfo
> {
>    uint flags;           // Module flags
>    uint index;           // Index into moduleinfo array
>    void*[14] reserved;   // Padding large enough to contain
>                          // all optional data added depending on
> flags.
> }
> 
> This however is not really required, but am just throughing it out there as a thought.

ModuleInfo could really need a cleanup, sounds good.
> 
> 
> Any thoughts on this?  (Looking at you Johannes).
> 
> Regards
> Iain

Everything sounds good so far. As far as I can tell the workarounds we implemented some time ago work fine, but it would certainly be good to implement the proper fixes.

Maybe we can revert some of this code after this
fix:
git log --grep=fsection-anchors

On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:

> Am Tue, 23 Apr 2013 17:10:43 +0200
> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
>
> > In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
> >
> >
> > I'm currently working on dealing with each of these issues in the
> > following pull (with the intention to merge back upstream where
> > required).
> > https://github.com/D-Programming-GDC/GDC/pull/62
> >
> I'll make sure to have a look at this, but as always too little time...
>
> > In order:
> >
> > 1. ClassInfo
> >
> > The initialiser emitted will have two symbols, one public symbol with the TypeInfo_Class members, and a second private generated symbol for the interfaces array.  I can't forsee any way this could break compatibility with any existing compilied gdc (or perhaps even dmd/ldc) binaries out there.
>
> Sounds good. That should also be good for debug info.
>
>
Actually, found an interesting problem when confronting it.

---
struct Interface
{
    TypeInfo_Class   classinfo;
    void*[]     vtbl;
    ptrdiff_t   offset;     /// offset to Interface 'this' from Object
'this'  <--
}
---

I'll have to have a think about what value should be put there, and how best to put it in.

-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';

On 23 April 2013 18:25, Iain Buclaw <ibuclaw@ubuntu.com> wrote:

> On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:
>
>> Am Tue, 23 Apr 2013 17:10:43 +0200
>> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
>>
>> > In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
>> >
>> >
>> > I'm currently working on dealing with each of these issues in the
>> > following pull (with the intention to merge back upstream where
>> > required).
>> > https://github.com/D-Programming-GDC/GDC/pull/62
>> >
>> I'll make sure to have a look at this, but as always too little time...
>>
>> > In order:
>> >
>> > 1. ClassInfo
>> >
>> > The initialiser emitted will have two symbols, one public symbol with the TypeInfo_Class members, and a second private generated symbol for the interfaces array.  I can't forsee any way this could break compatibility with any existing compilied gdc (or perhaps even dmd/ldc) binaries out there.
>>
>> Sounds good. That should also be good for debug info.
>>
>>
> Actually, found an interesting problem when confronting it.
>
> ---
> struct Interface
> {
>     TypeInfo_Class   classinfo;
>     void*[]     vtbl;
>     ptrdiff_t   offset;     /// offset to Interface 'this' from Object
> 'this'  <--
> }
> ---
>
> I'll have to have a think about what value should be put there, and how best to put it in.
>
>

Or alternatively we could generate the type (for debugging) on the flying
in ::toSymbol

1.  Module::toSymbol.

Type = Type::moduleinfo type.


2. ClassDeclaration::toSymbol

Type = Type::typeinfoclass type.

Then for each interface, add the fields for Type::interface onto the type.


3. InterfaceDeclaration::toSymbol

Likewise to above.


Regards
-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';

On 24 April 2013 15:12, Iain Buclaw <ibuclaw@ubuntu.com> wrote:

> On 23 April 2013 18:25, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
>
>> On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:
>>
>>> Am Tue, 23 Apr 2013 17:10:43 +0200
>>> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
>>>
>>> > In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
>>> >
>>> >
>>> > I'm currently working on dealing with each of these issues in the
>>> > following pull (with the intention to merge back upstream where
>>> > required).
>>> > https://github.com/D-Programming-GDC/GDC/pull/62
>>> >
>>> I'll make sure to have a look at this, but as always too little time...
>>>
>>> > In order:
>>> >
>>> > 1. ClassInfo
>>> >
>>> > The initialiser emitted will have two symbols, one public symbol with the TypeInfo_Class members, and a second private generated symbol for the interfaces array.  I can't forsee any way this could break compatibility with any existing compilied gdc (or perhaps even dmd/ldc) binaries out there.
>>>
>>> Sounds good. That should also be good for debug info.
>>>
>>>
>> Actually, found an interesting problem when confronting it.
>>
>> ---
>> struct Interface
>> {
>>     TypeInfo_Class   classinfo;
>>     void*[]     vtbl;
>>     ptrdiff_t   offset;     /// offset to Interface 'this' from Object
>> 'this'  <--
>> }
>> ---
>>
>> I'll have to have a think about what value should be put there, and how best to put it in.
>>
>>
>
> Or alternatively we could generate the type (for debugging) on the flying
> in ::toSymbol
>
> 1.  Module::toSymbol.
>
> Type = Type::moduleinfo type.
>
>
> 2. ClassDeclaration::toSymbol
>
> Type = Type::typeinfoclass type.
>
> Then for each interface, add the fields for Type::interface onto the type.
>
>
> 3. InterfaceDeclaration::toSymbol
>
> Likewise to above.
>
>
For the time being in this pull:

https://github.com/D-Programming-GDC/GDC/pull/62

For ModuleInfo, TypeInfoClass and Interface classes, I have added a new kind of type node into the glue layer.

- d_unknown_type_node: LANG_TYPE

This means that the backend won't touch, or do any sort of laying out of the type.

If we find this type in d_finalize_symbol  (formerly called outdata), then we give it the compiler generated type of the initialiser, and for debug purposes give it an eponymous type name  (eg:  _Dobject_ModuleInfoZ symbol has type name struct _Dobject_ModuleInfoZ).

This is fine for the workaround so far, but future improvements would be to take the logic from toObjFile / genmoduleinfo and give the fields the correct names when generating the symbol in toSymbol.

Alternatively we could store this information within Symbol itself during the toObjFile / genmoduleinfo stage, and have d_finalize_symbol call a routine that will generate the correct type layout to prevent code duplication.


Regards
-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';

On Wednesday, 8 May 2013 at 11:34:19 UTC, Iain Buclaw wrote:
> On 24 April 2013 15:12, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
>
>> On 23 April 2013 18:25, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
>>
>>> On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:
>>>
>>>> Am Tue, 23 Apr 2013 17:10:43 +0200
>>>> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
>>>>
>>>> > In reference to this link:
>>>> > http://forum.dlang.org/post/50476C77.20608@googlemail.com
>>>> >
>>>> >
>>>> > I'm currently working on dealing with each of these issues in the
>>>> > following pull (with the intention to merge back upstream where
>>>> > required).
>>>> > https://github.com/D-Programming-GDC/GDC/pull/62
>>>> >
>>>> I'll make sure to have a look at this, but as always too little time...
>>>>
>>>> > In order:
>>>> >
>>>> > 1. ClassInfo
>>>> >
>>>> > The initialiser emitted will have two symbols, one public symbol
>>>> > with the TypeInfo_Class members, and a second private generated
>>>> > symbol for the interfaces array.  I can't forsee any way this
>>>> > could break compatibility with any existing compilied gdc (or
>>>> > perhaps even dmd/ldc) binaries out there.
>>>>
>>>> Sounds good. That should also be good for debug info.
>>>>
>>>>
>>> Actually, found an interesting problem when confronting it.
>>>
>>> ---
>>> struct Interface
>>> {
>>>     TypeInfo_Class   classinfo;
>>>     void*[]     vtbl;
>>>     ptrdiff_t   offset;     /// offset to Interface 'this' from Object
>>> 'this'  <--
>>> }
>>> ---
>>>
>>> I'll have to have a think about what value should be put there, and how
>>> best to put it in.
>>>
>>>
>>
>> Or alternatively we could generate the type (for debugging) on the flying
>> in ::toSymbol
>>
>> 1.  Module::toSymbol.
>>
>> Type = Type::moduleinfo type.
>>
>>
>> 2. ClassDeclaration::toSymbol
>>
>> Type = Type::typeinfoclass type.
>>
>> Then for each interface, add the fields for Type::interface onto the type.
>>
>>
>> 3. InterfaceDeclaration::toSymbol
>>
>> Likewise to above.
>>
>>
> For the time being in this pull:
>
> https://github.com/D-Programming-GDC/GDC/pull/62
>
> For ModuleInfo, TypeInfoClass and Interface classes, I have added a new
> kind of type node into the glue layer.
>
> - d_unknown_type_node: LANG_TYPE
>
> This means that the backend won't touch, or do any sort of laying out of
> the type.
>
> If we find this type in d_finalize_symbol  (formerly called outdata), then
> we give it the compiler generated type of the initialiser, and for debug
> purposes give it an eponymous type name  (eg:  _Dobject_ModuleInfoZ symbol
> has type name struct _Dobject_ModuleInfoZ).
>
> This is fine for the workaround so far, but future improvements would be to
> take the logic from toObjFile / genmoduleinfo and give the fields the
> correct names when generating the symbol in toSymbol.
>
> Alternatively we could store this information within Symbol itself during
> the toObjFile / genmoduleinfo stage, and have d_finalize_symbol call a
> routine that will generate the correct type layout to prevent code
> duplication.
>
>


Also, I have altered the mismatch check in d_finalize_symbol (outdata) that you put in initially for this fix.  Now it asserts that type_size == initialiser_type_size.

This passes through the testsuite 100%, so I'll keep it.  There should be no reason whatsoever why the initialiser would be less than the type size.

Regards
Iain

Am Thu, 09 May 2013 19:40:04 +0200
schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:

> On Wednesday, 8 May 2013 at 11:34:19 UTC, Iain Buclaw wrote:
> > On 24 April 2013 15:12, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
> >
> >> On 23 April 2013 18:25, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
> >>
> >>> On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:
> >>>
> >>>> Am Tue, 23 Apr 2013 17:10:43 +0200
> >>>> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
> >>>>
> >>>> > In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
> >>>> >
> >>>> >
> >>>> > I'm currently working on dealing with each of these issues
> >>>> > in the
> >>>> > following pull (with the intention to merge back upstream
> >>>> > where
> >>>> > required).
> >>>> > https://github.com/D-Programming-GDC/GDC/pull/62
> >>>> >
> >>>> I'll make sure to have a look at this, but as always too little time...
> >>>>
> >>>> > In order:
> >>>> >
> >>>> > 1. ClassInfo
> >>>> >
> >>>> > The initialiser emitted will have two symbols, one public
> >>>> > symbol
> >>>> > with the TypeInfo_Class members, and a second private
> >>>> > generated
> >>>> > symbol for the interfaces array.  I can't forsee any way
> >>>> > this
> >>>> > could break compatibility with any existing compilied gdc
> >>>> > (or
> >>>> > perhaps even dmd/ldc) binaries out there.
> >>>>
> >>>> Sounds good. That should also be good for debug info.
> >>>>
> >>>>
> >>> Actually, found an interesting problem when confronting it.
> >>>
> >>> ---
> >>> struct Interface
> >>> {
> >>>     TypeInfo_Class   classinfo;
> >>>     void*[]     vtbl;
> >>>     ptrdiff_t   offset;     /// offset to Interface 'this'
> >>> from Object
> >>> 'this'  <--
> >>> }
> >>> ---
> >>>
> >>> I'll have to have a think about what value should be put
> >>> there, and how
> >>> best to put it in.
> >>>
> >>>
> >>
> >> Or alternatively we could generate the type (for debugging) on
> >> the flying
> >> in ::toSymbol
> >>
> >> 1.  Module::toSymbol.
> >>
> >> Type = Type::moduleinfo type.
> >>
> >>
> >> 2. ClassDeclaration::toSymbol
> >>
> >> Type = Type::typeinfoclass type.
> >>
> >> Then for each interface, add the fields for Type::interface onto the type.
> >>
> >>
> >> 3. InterfaceDeclaration::toSymbol
> >>
> >> Likewise to above.
> >>
> >>
> > For the time being in this pull:
> >
> > https://github.com/D-Programming-GDC/GDC/pull/62
> >
> > For ModuleInfo, TypeInfoClass and Interface classes, I have
> > added a new
> > kind of type node into the glue layer.
> >
> > - d_unknown_type_node: LANG_TYPE
> >
> > This means that the backend won't touch, or do any sort of
> > laying out of
> > the type.
> >
> > If we find this type in d_finalize_symbol  (formerly called
> > outdata), then
> > we give it the compiler generated type of the initialiser, and
> > for debug
> > purposes give it an eponymous type name  (eg:
> > _Dobject_ModuleInfoZ symbol
> > has type name struct _Dobject_ModuleInfoZ).
> >
> > This is fine for the workaround so far, but future improvements
> > would be to
> > take the logic from toObjFile / genmoduleinfo and give the
> > fields the
> > correct names when generating the symbol in toSymbol.
> >
> > Alternatively we could store this information within Symbol
> > itself during
> > the toObjFile / genmoduleinfo stage, and have d_finalize_symbol
> > call a
> > routine that will generate the correct type layout to prevent
> > code
> > duplication.
> >
> >
> 
> 
> Also, I have altered the mismatch check in d_finalize_symbol (outdata) that you put in initially for this fix.  Now it asserts that type_size == initialiser_type_size.
> 
> This passes through the testsuite 100%, so I'll keep it.  There should be no reason whatsoever why the initialiser would be less than the type size.
> 

If it passes the testsuite then there is indeed no reason not to change it. Not sure why I made it check for >= in the first place.

On 11 May 2013 12:50, Johannes Pfau <nospam@example.com> wrote:

> Am Thu, 09 May 2013 19:40:04 +0200
> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
>
> > On Wednesday, 8 May 2013 at 11:34:19 UTC, Iain Buclaw wrote:
> > > On 24 April 2013 15:12, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
> > >
> > >> On 23 April 2013 18:25, Iain Buclaw <ibuclaw@ubuntu.com> wrote:
> > >>
> > >>> On 23 April 2013 17:28, Johannes Pfau <nospam@example.com> wrote:
> > >>>
> > >>>> Am Tue, 23 Apr 2013 17:10:43 +0200
> > >>>> schrieb "Iain Buclaw" <ibuclaw@ubuntu.com>:
> > >>>>
> > >>>> > In reference to this link: http://forum.dlang.org/post/50476C77.20608@googlemail.com
> > >>>> >
> > >>>> >
> > >>>> > I'm currently working on dealing with each of these issues
> > >>>> > in the
> > >>>> > following pull (with the intention to merge back upstream
> > >>>> > where
> > >>>> > required).
> > >>>> > https://github.com/D-Programming-GDC/GDC/pull/62
> > >>>> >
> > >>>> I'll make sure to have a look at this, but as always too little time...
> > >>>>
> > >>>> > In order:
> > >>>> >
> > >>>> > 1. ClassInfo
> > >>>> >
> > >>>> > The initialiser emitted will have two symbols, one public
> > >>>> > symbol
> > >>>> > with the TypeInfo_Class members, and a second private
> > >>>> > generated
> > >>>> > symbol for the interfaces array.  I can't forsee any way
> > >>>> > this
> > >>>> > could break compatibility with any existing compilied gdc
> > >>>> > (or
> > >>>> > perhaps even dmd/ldc) binaries out there.
> > >>>>
> > >>>> Sounds good. That should also be good for debug info.
> > >>>>
> > >>>>
> > >>> Actually, found an interesting problem when confronting it.
> > >>>
> > >>> ---
> > >>> struct Interface
> > >>> {
> > >>>     TypeInfo_Class   classinfo;
> > >>>     void*[]     vtbl;
> > >>>     ptrdiff_t   offset;     /// offset to Interface 'this'
> > >>> from Object
> > >>> 'this'  <--
> > >>> }
> > >>> ---
> > >>>
> > >>> I'll have to have a think about what value should be put
> > >>> there, and how
> > >>> best to put it in.
> > >>>
> > >>>
> > >>
> > >> Or alternatively we could generate the type (for debugging) on
> > >> the flying
> > >> in ::toSymbol
> > >>
> > >> 1.  Module::toSymbol.
> > >>
> > >> Type = Type::moduleinfo type.
> > >>
> > >>
> > >> 2. ClassDeclaration::toSymbol
> > >>
> > >> Type = Type::typeinfoclass type.
> > >>
> > >> Then for each interface, add the fields for Type::interface onto the type.
> > >>
> > >>
> > >> 3. InterfaceDeclaration::toSymbol
> > >>
> > >> Likewise to above.
> > >>
> > >>
> > > For the time being in this pull:
> > >
> > > https://github.com/D-Programming-GDC/GDC/pull/62
> > >
> > > For ModuleInfo, TypeInfoClass and Interface classes, I have
> > > added a new
> > > kind of type node into the glue layer.
> > >
> > > - d_unknown_type_node: LANG_TYPE
> > >
> > > This means that the backend won't touch, or do any sort of
> > > laying out of
> > > the type.
> > >
> > > If we find this type in d_finalize_symbol  (formerly called
> > > outdata), then
> > > we give it the compiler generated type of the initialiser, and
> > > for debug
> > > purposes give it an eponymous type name  (eg:
> > > _Dobject_ModuleInfoZ symbol
> > > has type name struct _Dobject_ModuleInfoZ).
> > >
> > > This is fine for the workaround so far, but future improvements
> > > would be to
> > > take the logic from toObjFile / genmoduleinfo and give the
> > > fields the
> > > correct names when generating the symbol in toSymbol.
> > >
> > > Alternatively we could store this information within Symbol
> > > itself during
> > > the toObjFile / genmoduleinfo stage, and have d_finalize_symbol
> > > call a
> > > routine that will generate the correct type layout to prevent
> > > code
> > > duplication.
> > >
> > >
> >
> >
> > Also, I have altered the mismatch check in d_finalize_symbol (outdata) that you put in initially for this fix.  Now it asserts that type_size == initialiser_type_size.
> >
> > This passes through the testsuite 100%, so I'll keep it.  There should be no reason whatsoever why the initialiser would be less than the type size.
> >
>
> If it passes the testsuite then there is indeed no reason not to change it. Not sure why I made it check for >= in the first place.
>

If decl_initial size  < type size. Then the backend will always automatically pad out the end with zeroes.


-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';