[lld] r247165 - ELF2: Return early. NFC.

[David Blaikie via llvm-commits]

On Wed, Sep 9, 2015 at 10:40 AM, Rui Ueyama via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: ruiu
> Date: Wed Sep  9 12:40:51 2015
> New Revision: 247165
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247165&view=rev
> Log:
> ELF2: Return early. NFC.
>
> Modified:
>     lld/trunk/ELF/Symbols.cpp
>
> Modified: lld/trunk/ELF/Symbols.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.cpp?rev=247165&r1=247164&r2=247165&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/Symbols.cpp (original)
> +++ lld/trunk/ELF/Symbols.cpp Wed Sep  9 12:40:51 2015
> @@ -51,27 +51,26 @@ template <class ELFT> int SymbolBody::co
>    if (L != R)
>      return -1;
>
> -  if (L.first && L.second) {
> -    if (isCommon()) {
> -      if (Other->isCommon()) {
> -        auto *ThisC = cast<DefinedCommon<ELFT>>(this);
> -        auto *OtherC = cast<DefinedCommon<ELFT>>(Other);
> -        typename DefinedCommon<ELFT>::uintX_t MaxAlign =
> -            std::max(ThisC->MaxAlignment, OtherC->MaxAlignment);
> -        if (ThisC->Sym.st_size >= OtherC->Sym.st_size) {
> -          ThisC->MaxAlignment = MaxAlign;
> -          return 1;
> -        }
> -        OtherC->MaxAlignment = MaxAlign;
> -        return -1;
> +  if (!L.first || !L.second)
> +    return 1;
> +  if (isCommon()) {
> +    if (Other->isCommon()) {
> +      auto *ThisC = cast<DefinedCommon<ELFT>>(this);
> +      auto *OtherC = cast<DefinedCommon<ELFT>>(Other);
> +      typename DefinedCommon<ELFT>::uintX_t MaxAlign =
> +          std::max(ThisC->MaxAlignment, OtherC->MaxAlignment);
> +      if (ThisC->Sym.st_size >= OtherC->Sym.st_size) {
> +        ThisC->MaxAlignment = MaxAlign;
> +        return 1;
>        }
> +      OtherC->MaxAlignment = MaxAlign;
>        return -1;
>

You could further reduce some nesting by inverting this block ^. Instead of:

if (Other->isCommon()) {
  ...
  return -1;
}
return -1;

You could:

if (!Other->isCommon())|
  return -1;
...

(alternatively, if you prefer the nesting, you could at least drop the
nested return -1, it'll exit the if and immediately return -1 anyway)


>      }
> -    if (Other->isCommon())
> -      return 1;
> -    return 0;
> +    return -1;
>    }
> -  return 1;
> +  if (Other->isCommon())
> +    return 1;
> +  return 0;
>

Possibly not more readable, but this could be written as "return
Other->isCommon()".


>  }
>
>  std::unique_ptr<InputFile> Lazy::getMember() {
>
>
> _______________________________________________
> 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/20150909/b1f8b86c/attachment.html>