[lld] r319922 - Add a checkLazy error checking variant.

[Rui Ueyama via llvm-commits]

I want to keep the number of functions in ErrorHandler as small as possible
for the sake of simplicity. As you added a new function, now we have to
think which we should use when we write code, check or checkLazy.

Have you considered making every call of `check` lazy? I'm OK with using C
macro if not too complicated.

Also, lazy is a overloaded word in lld, so it is better to avoid that word
if it's not related to the lazy symbol.

On Wed, Dec 6, 2017 at 10:52 AM, Rafael Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rafael
> Date: Wed Dec  6 10:52:13 2017
> New Revision: 319922
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319922&view=rev
> Log:
> Add a checkLazy error checking variant.
>
> This avoids allocating the error message when there is no error that
> check requires.
>
> It avoids the code duplication of inlining check.
>
> Modified:
>     lld/trunk/ELF/InputFiles.cpp
>     lld/trunk/include/lld/Common/ErrorHandler.h
>
> Modified: lld/trunk/ELF/InputFiles.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> InputFiles.cpp?rev=319922&r1=319921&r2=319922&view=diff
> ============================================================
> ==================
> --- lld/trunk/ELF/InputFiles.cpp (original)
> +++ lld/trunk/ELF/InputFiles.cpp Wed Dec  6 10:52:13 2017
> @@ -209,10 +209,8 @@ typename ELFT::SymRange ELFFileBase<ELFT
>
>  template <class ELFT>
>  uint32_t ELFFileBase<ELFT>::getSectionIndex(const Elf_Sym &Sym) const {
> -  auto RetOrErr = getObj().getSectionIndex(&Sym, ELFSyms, SymtabSHNDX);
> -  if (RetOrErr)
> -    return *RetOrErr;
> -  fatal(toString(this) + ": " + toString(RetOrErr.takeError()));
> +  return checkLazy(getObj().getSectionIndex(&Sym, ELFSyms, SymtabSHNDX),
> +                   [=]() { return toString(this); });
>  }
>
>  template <class ELFT>
> @@ -642,10 +640,8 @@ template <class ELFT> Symbol *ObjFile<EL
>      return make<Defined>(this, Name, Binding, StOther, Type, Value, Size,
> Sec);
>    }
>
> -  auto NameOrErr = Sym->getName(this->StringTable);
> -  if (!NameOrErr)
> -    fatal(toString(this) + ": " + toString(NameOrErr.takeError()));
> -  StringRef Name = *NameOrErr;
> +  StringRef Name = checkLazy(Sym->getName(this->StringTable),
> +                             [=]() { return toString(this); });
>
>    switch (Sym->st_shndx) {
>    case SHN_UNDEF:
>
> Modified: lld/trunk/include/lld/Common/ErrorHandler.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/
> Common/ErrorHandler.h?rev=319922&r1=319921&r2=319922&view=diff
> ============================================================
> ==================
> --- lld/trunk/include/lld/Common/ErrorHandler.h (original)
> +++ lld/trunk/include/lld/Common/ErrorHandler.h Wed Dec  6 10:52:13 2017
> @@ -30,6 +30,7 @@
>
>  #include "lld/Common/LLVM.h"
>
> +#include "llvm/ADT/STLExtras.h"
>  #include "llvm/Support/Error.h"
>  #include "llvm/Support/FileOutputBuffer.h"
>
> @@ -99,6 +100,13 @@ template <class T> T check(Expected<T> E
>    return std::move(*E);
>  }
>
> +// A lazy variant that only allocates error messages when there is an
> error.
> +template <class T>
> +T checkLazy(Expected<T> E, llvm::function_ref<std::string()> getPrefix) {
> +  if (!E)
> +    fatal(getPrefix() + ": " + toString(E.takeError()));
> +  return std::move(*E);
> +}
>  } // namespace lld
>
>  #endif
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171206/59b7efb3/attachment.html>