Bas van den Berg
2014-Jun-02 07:44 UTC
[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>
Michael Spencer
2014-Jun-02 20:05 UTC
[LLVMdev] [lld] LLD's software architecture (update)
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 >