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

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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/eaa5b5af/attachment.html>

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/11c9195c/attachment.html>

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>