[PATCH] D22506: [ELF] - Cleanup of LinkerScript<ELFT>::assignAddresses()

[Rafael Espíndola via llvm-commits]

If you make it a member function you can return void and set Dot when
appropriate. Would that be better?

Cheers,
Rafael


On 19 July 2016 at 05:11, George Rimar <grimar at accesssoftek.com> wrote:
> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar, evgeny777.
>
> I suggest this cleanup change because of two reasons:
> 1) LinkerScript<ELFT>::assignAddresses is becoming larger and looks it can be good time for splitting.
> 2) I expect to see more SectionsCommand's there, and dispatching some of them separatelly can help to keep method smaller either.
>
> https://reviews.llvm.org/D22506
>
> Files:
>   ELF/LinkerScript.cpp
>
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -207,6 +207,19 @@
>  }
>
>  template <class ELFT>
> +static typename ELFT::uint dispatchAssignment(SectionsCommand &Cmd,
> +                                              typename ELFT::uint Dot) {
> +  uint64_t Val = evalExpr(Cmd.Expr, Dot);
> +  if (Cmd.Name == ".") {
> +    Dot = Val;
> +  } else {
> +    auto *D = cast<DefinedRegular<ELFT>>(Symtab<ELFT>::X->find(Cmd.Name));
> +    D->Value = Val;
> +  }
> +  return Dot;
> +}
> +
> +template <class ELFT>
>  void LinkerScript<ELFT>::assignAddresses(
>      ArrayRef<OutputSectionBase<ELFT> *> Sections) {
>    // Orphan sections are sections present in the input files which
> @@ -226,22 +239,15 @@
>    uintX_t ThreadBssOffset = 0;
>
>    for (SectionsCommand &Cmd : Opt.Commands) {
> -    if (Cmd.Kind == AssignmentKind) {
> -      uint64_t Val = evalExpr(Cmd.Expr, Dot);
> +    if (Cmd.Kind == AssignmentKind)
> +      Dot = dispatchAssignment<ELFT>(Cmd, Dot);
>
> -      if (Cmd.Name == ".") {
> -        Dot = Val;
> -      } else {
> -        auto *D = cast<DefinedRegular<ELFT>>(Symtab<ELFT>::X->find(Cmd.Name));
> -        D->Value = Val;
> -      }
> +    if (Cmd.Kind != SectionKind)
>        continue;
> -    }
>
>      // Find all the sections with required name. There can be more than
>      // one section with such name, if the alignment, flags or type
>      // attribute differs.
> -    assert(Cmd.Kind == SectionKind);
>      for (OutputSectionBase<ELFT> *Sec : Sections) {
>        if (Sec->getName() != Cmd.Name)
>          continue;
>
>