[PATCH] D37477: [ELF] - Linkerscript: implement REGION_ALIAS.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 7 12:22:13 PDT 2017


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: ELF/ScriptParser.cpp:1240-1242
+    MemoryRegion *MR = make<MemoryRegion>();
+    *MR = {Name, Origin, Length, Flags, NegFlags};
+    Script->Opt.MemoryRegions[Name] = MR;
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > ruiu wrote:
> > > > Does this work?
> > > > 
> > > >   Script->Opt.MemoryRegions[Name] =
> > > >     make<MemoryRegion>(Name, Origin, Length, Flags, NegFlags);
> > > Nope. Error is: "'lld::elf::MemoryRegion::MemoryRegion': no overloaded function takes 5 arguments",
> > > that is why I did not do that initially. I believe it will work if we add constructor taking these 5 to
> > > `MemoryRegion`, but does not seems it really worth to do for me.
> > Then how about this?
> > 
> >    Script->Opt.MemoryRegions[Name] =
> >      make<MemoryRegion>({Name, Origin, Length, Flags, NegFlags});
> That was first I tried initially, it will error out:
> error C2660: 'lld::elf::make': function does not take 1 arguments
> 
> As I mentioned, with constructor taking this arguments it is possible to use
> ```
>     Script->Opt.MemoryRegions[Name] =
>       make<MemoryRegion>(Name, Origin, Length, Flags, NegFlags);
> ```
> 
> Do you want me to add constructor ?
> Do you want me to add constructor ?

No, as it's not overall a win.


https://reviews.llvm.org/D37477





More information about the llvm-commits mailing list