[PATCH] D30163: [ELF] - Postpone evaluation of LMA offset.

[Rui Ueyama via llvm-commits]

On Wed, Feb 22, 2017 at 1:50 AM, George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added inline comments.
>
>
> ================
> Comment at: lld/trunk/ELF/LinkerScript.cpp:568
>    if (Cmd->LMAExpr)
> -    LMAOffset = Cmd->LMAExpr(Dot) - Dot;
> +    CurLMA = {Cmd->LMAExpr, Dot};
>    OutputSectionBase *Sec = findSection<ELFT>(Cmd->Name, *OutputSections);
> ----------------
> ruiu wrote:
> > If you change this to
> >
> >   LMAOffset = [=] { return Cmd->LMAExpr(Dot) - Dot; }
> >
> > I generally don't like to overuse std::pair because its member names
> (first and second) sometimes hurts code readability.
> That would work if I do something like:
>
> ```
>   if (Cmd->LMAExpr) {
>     uintX_t D = Dot;
>     LMAOffset = [=] { return Cmd->LMAExpr(D) - D; };
>   }
> ```
>
> Because you can't capture member Dot by value.
> Does it look fine for you ?
>

Yes, I think it's better. In C++14 you can write it as `[D= Dot] { return
Cmd->LMAExpr(D) - D; }`, so there's an easy upgrade path forward. :)


> ================
> Comment at: lld/trunk/ELF/LinkerScript.h:301
>    uintX_t Dot;
> -  uintX_t LMAOffset = 0;
> +  std::pair<Expr, uintX_t> CurLMA;
>    OutputSectionBase *CurOutSec = nullptr;
> ----------------
> ruiu wrote:
> > This can be just `Expr LMAOffset`, right?
> >
> I would use std::function<uint64_t()> then. Expr assumes Dot argument
> which is unused.
> And would complicate the initialization to:
>
> ```
> LMAOffset = {[=](uint64_t) { return Cmd->LMAExpr(D) - D; }};
> ```
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D30163
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170222/c32f0e97/attachment.html>