llvm dev - Jun 2014 - [LLVMdev] [lld] LLD's software architecture (update)

The inverted dependency of Core to ReaderWriter via Simple.h was already
present.
My idea was to fix the problem before it gets bigger.
My proposal would be to move Simple.h and Alias.h to Core. Similar to
UndefinedAtom.h etc.
It would be even nicer to make the naming consistent as well, since there
already is
UndefinedAtom.h
SharedLibaryAtom.h
etc

Maybe:
SimpleAtom.h
AliasAtom.h

Moving the file to core fixes the dependencies.




On Mon, Jun 2, 2014 at 9:25 AM, Rui Ueyama <ruiu at google.com> wrote:
> On Mon, Jun 2, 2014 at 12:01 AM, Bas van den Berg <
> b.van.den.berg.nl at gmail.com> wrote:
>
>> Hi,
>>
>> I've been following the changes in LLD's software architecture.
>> Recently a new file was added: Alias.h
>> In this file, the curret Atom set is extended with an AliasAtom.
>> While the change seems innocent enough, it has some nasty
>> potential. Simple.h and Alias.h are placed in the ReaderWriter
>> component. This is the 2nd component in the layering:
>> Driver
>> ReaderWriter
>> Passes
>> Core
>>
>> The problem is that Core depends on these files. So in my opinion,
>> they belong to that Component. The greater impact here is that
>> once core starts depending on ReaderWriter, architecture Erosion
>> will happen. This would obviously not be a good thing, since the
>> current design is fresh and clean.
>>
>
> Core's dependency to Simple.h was there before the addition of Alias.h,
so
> it's not new. And I don't feel like it's that bad as you might
be feeling.
> Do you have any suggestion to get rid of it? Just moving the file to Core?
>
> I would love to supply patches, but am having issues getting
>> lld to build under Ubuntu 12.04. (but that's another story ;) ).
>>
>> I've posted the updated architecture view here:
>> http://www.c2lang.org/docs/lld_architecture_20140602.png
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/a4a02fe7/attachment.html>

I agree to move these files to Core. Any objections?

On Mon, Jun 2, 2014 at 12:44 AM, Bas van den Berg <
b.van.den.berg.nl at gmail.com> wrote:
> The inverted dependency of Core to ReaderWriter via Simple.h was already
> present.
> My idea was to fix the problem before it gets bigger.
> My proposal would be to move Simple.h and Alias.h to Core. Similar to
> UndefinedAtom.h etc.
> It would be even nicer to make the naming consistent as well, since there
> already is
> UndefinedAtom.h
> SharedLibaryAtom.h
> etc
>
> Maybe:
> SimpleAtom.h
>  AliasAtom.h
>
> Moving the file to core fixes the dependencies.
>
>
>
>
> On Mon, Jun 2, 2014 at 9:25 AM, Rui Ueyama <ruiu at google.com>
wrote:
>
>> On Mon, Jun 2, 2014 at 12:01 AM, Bas van den Berg <
>> b.van.den.berg.nl at gmail.com> wrote:
>>
>>> Hi,
>>>
>>> I've been following the changes in LLD's software
architecture.
>>> Recently a new file was added: Alias.h
>>> In this file, the curret Atom set is extended with an AliasAtom.
>>> While the change seems innocent enough, it has some nasty
>>> potential. Simple.h and Alias.h are placed in the ReaderWriter
>>> component. This is the 2nd component in the layering:
>>> Driver
>>> ReaderWriter
>>> Passes
>>> Core
>>>
>>> The problem is that Core depends on these files. So in my opinion,
>>> they belong to that Component. The greater impact here is that
>>> once core starts depending on ReaderWriter, architecture Erosion
>>> will happen. This would obviously not be a good thing, since the
>>> current design is fresh and clean.
>>>
>>
>> Core's dependency to Simple.h was there before the addition of
Alias.h,
>> so it's not new. And I don't feel like it's that bad as you
might be
>> feeling. Do you have any suggestion to get rid of it? Just moving the
file
>> to Core?
>>
>> I would love to supply patches, but am having issues getting
>>> lld to build under Ubuntu 12.04. (but that's another story ;)
).
>>>
>>> I've posted the updated architecture view here:
>>> http://www.c2lang.org/docs/lld_architecture_20140602.png
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/6400e0ce/attachment.html>

On Mon, Jun 2, 2014 at 12:49 AM, Rui Ueyama <ruiu at google.com>
wrote:
> I agree to move these files to Core. Any objections?
None here.

- Michael Spencer
>
> On Mon, Jun 2, 2014 at 12:44 AM, Bas van den Berg
> <b.van.den.berg.nl at gmail.com> wrote:
>>
>> The inverted dependency of Core to ReaderWriter via Simple.h was
already
>> present.
>> My idea was to fix the problem before it gets bigger.
>> My proposal would be to move Simple.h and Alias.h to Core. Similar to
>> UndefinedAtom.h etc.
>> It would be even nicer to make the naming consistent as well, since
there
>> already is
>> UndefinedAtom.h
>> SharedLibaryAtom.h
>> etc
>>
>> Maybe:
>> SimpleAtom.h
>> AliasAtom.h
>>
>> Moving the file to core fixes the dependencies.
>>
>>
>>
>>
>> On Mon, Jun 2, 2014 at 9:25 AM, Rui Ueyama <ruiu at google.com>
wrote:
>>>
>>> On Mon, Jun 2, 2014 at 12:01 AM, Bas van den Berg
>>> <b.van.den.berg.nl at gmail.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I've been following the changes in LLD's software
architecture.
>>>> Recently a new file was added: Alias.h
>>>> In this file, the curret Atom set is extended with an
AliasAtom.
>>>> While the change seems innocent enough, it has some nasty
>>>> potential. Simple.h and Alias.h are placed in the ReaderWriter
>>>> component. This is the 2nd component in the layering:
>>>> Driver
>>>> ReaderWriter
>>>> Passes
>>>> Core
>>>>
>>>> The problem is that Core depends on these files. So in my
opinion,
>>>> they belong to that Component. The greater impact here is that
>>>> once core starts depending on ReaderWriter, architecture
Erosion
>>>> will happen. This would obviously not be a good thing, since
the
>>>> current design is fresh and clean.
>>>
>>>
>>> Core's dependency to Simple.h was there before the addition of
Alias.h,
>>> so it's not new. And I don't feel like it's that bad as
you might be
>>> feeling. Do you have any suggestion to get rid of it? Just moving
the file
>>> to Core?
>>>
>>>> I would love to supply patches, but am having issues getting
>>>> lld to build under Ubuntu 12.04. (but that's another story
;) ).
>>>>
>>>> I've posted the updated architecture view here:
>>>> http://www.c2lang.org/docs/lld_architecture_20140602.png
>>>>
>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>
>>>
>>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>