[PATCH] D19111: [ELF] - Remove OutputSectionFactory::lookup method

[Rafael Espíndola via llvm-commits]

How about just defining it inline?

One of the annoyances of the Map interface is the just using [] adds
an entry. Having the Map be private means we only have to look at a
few places to know that we don't accidentally create entries when we
don't want to: the two visible methods are named create and lookup.

Cheers,
Rafael


On 14 April 2016 at 09:25, George Rimar <grimar at accesssoftek.com> wrote:
> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar.
>
> What about removing OutputSectionFactory::lookup method ?
> It is single line and IMO removing will decrease amount
> of code without visible hurt.
>
> http://reviews.llvm.org/D19111
>
> Files:
>   ELF/Writer.cpp
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -996,13 +996,11 @@
>    std::pair<OutputSectionBase<ELFT> *, bool> create(InputSectionBase<ELFT> *C,
>                                                      StringRef OutsecName);
>
> -  OutputSectionBase<ELFT> *lookup(StringRef Name, uint32_t Type, uintX_t Flags);
> +  SmallDenseMap<SectionKey<ELFT::Is64Bits>, OutputSectionBase<ELFT> *> Map;
>
>  private:
>    SectionKey<ELFT::Is64Bits> createKey(InputSectionBase<ELFT> *C,
>                                         StringRef OutsecName);
> -
> -  SmallDenseMap<SectionKey<ELFT::Is64Bits>, OutputSectionBase<ELFT> *> Map;
>  };
>  }
>
> @@ -1034,13 +1032,6 @@
>  }
>
>  template <class ELFT>
> -OutputSectionBase<ELFT> *OutputSectionFactory<ELFT>::lookup(StringRef Name,
> -                                                            uint32_t Type,
> -                                                            uintX_t Flags) {
> -  return Map.lookup({Name, Type, Flags, 0});
> -}
> -
> -template <class ELFT>
>  SectionKey<ELFT::Is64Bits>
>  OutputSectionFactory<ELFT>::createKey(InputSectionBase<ELFT> *C,
>                                        StringRef OutsecName) {
> @@ -1177,25 +1168,26 @@
>    }
>
>    Out<ELFT>::Bss = static_cast<OutputSection<ELFT> *>(
> -      Factory.lookup(".bss", SHT_NOBITS, SHF_ALLOC | SHF_WRITE));
> +      Factory.Map.lookup({".bss", SHT_NOBITS, SHF_ALLOC | SHF_WRITE}));
>
>    // If we have a .opd section (used under PPC64 for function descriptors),
>    // store a pointer to it here so that we can use it later when processing
>    // relocations.
> -  Out<ELFT>::Opd = Factory.lookup(".opd", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC);
> +  Out<ELFT>::Opd =
> +      Factory.Map.lookup({".opd", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC});
>
> -  Out<ELFT>::Dynamic->PreInitArraySec = Factory.lookup(
> -      ".preinit_array", SHT_PREINIT_ARRAY, SHF_WRITE | SHF_ALLOC);
> +  Out<ELFT>::Dynamic->PreInitArraySec = Factory.Map.lookup({
> +    ".preinit_array", SHT_PREINIT_ARRAY, SHF_WRITE | SHF_ALLOC});
>    Out<ELFT>::Dynamic->InitArraySec =
> -      Factory.lookup(".init_array", SHT_INIT_ARRAY, SHF_WRITE | SHF_ALLOC);
> +    Factory.Map.lookup({ ".init_array", SHT_INIT_ARRAY, SHF_WRITE | SHF_ALLOC });
>    Out<ELFT>::Dynamic->FiniArraySec =
> -      Factory.lookup(".fini_array", SHT_FINI_ARRAY, SHF_WRITE | SHF_ALLOC);
> +    Factory.Map.lookup({ ".fini_array", SHT_FINI_ARRAY, SHF_WRITE | SHF_ALLOC });
>
>    // Sort section contents for __attribute__((init_priority(N)).
>    sortInitFini(Out<ELFT>::Dynamic->InitArraySec);
>    sortInitFini(Out<ELFT>::Dynamic->FiniArraySec);
> -  sortCtorsDtors(Factory.lookup(".ctors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
> -  sortCtorsDtors(Factory.lookup(".dtors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC));
> +  sortCtorsDtors(Factory.Map.lookup({".ctors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC}));
> +  sortCtorsDtors(Factory.Map.lookup({".dtors", SHT_PROGBITS, SHF_WRITE | SHF_ALLOC}));
>
>    // The linker needs to define SECNAME_start, SECNAME_end and SECNAME_stop
>    // symbols for sections, so that the runtime can get the start and end
>
>