[PATCH] Refactor: Simplify boolean expressions in llvm-nm

David Blaikie dblaikie at gmail.com
Sun Mar 22 13:45:17 PDT 2015


On Sun, Mar 22, 2015 at 12:39 PM, Richard <legalize at xmission.com> wrote:

> Hi rafael, alexfh,
>
> Simplify boolean expressions using `true` and `false` with `clang-tidy`
>
> http://reviews.llvm.org/D8518
>
> Files:
>   tools/llvm-nm/llvm-nm.cpp
>
> Index: tools/llvm-nm/llvm-nm.cpp
> ===================================================================
> --- tools/llvm-nm/llvm-nm.cpp
> +++ tools/llvm-nm/llvm-nm.cpp
> @@ -190,19 +190,15 @@
>        return true;
>      else if (A.Address == B.Address && A.Name < B.Name)
>        return true;
> -    else if (A.Address == B.Address && A.Name == B.Name && A.Size <
> B.Size)
> -      return true;
>      else
> -      return false;
> +      return A.Address == B.Address && A.Name == B.Name && A.Size <
> B.Size;
>

The asymmetry here (& in other cases in this patch) isn't ideal and this
code already violates the LLVM else-after-return coding convention.

If we changed the code to:

if (x)
  return true;
return false;

would it still trigger the cleanup check you're using? I don't mind
changing that last one to 'return x;' if that's substantially easier/better
than suppressing the check in such cases.


>    } else {
>      if (A.Address > B.Address)
>        return true;
>      else if (A.Address == B.Address && A.Name > B.Name)
>        return true;
> -    else if (A.Address == B.Address && A.Name == B.Name && A.Size >
> B.Size)
> -      return true;
>      else
> -      return false;
> +      return A.Address == B.Address && A.Name == B.Name && A.Size >
> B.Size;
>    }
>  }
>
> @@ -212,19 +208,15 @@
>        return true;
>      else if (A.Size == B.Size && A.Name < B.Name)
>        return true;
> -    else if (A.Size == B.Size && A.Name == B.Name && A.Address <
> B.Address)
> -      return true;
>      else
> -      return false;
> +      return A.Size == B.Size && A.Name == B.Name && A.Address <
> B.Address;
>    } else {
>      if (A.Size > B.Size)
>        return true;
>      else if (A.Size == B.Size && A.Name > B.Name)
>        return true;
> -    else if (A.Size == B.Size && A.Name == B.Name && A.Address >
> B.Address)
> -      return true;
>      else
> -      return false;
> +      return A.Size == B.Size && A.Name == B.Name && A.Address >
> B.Address;
>    }
>  }
>
> @@ -234,19 +226,15 @@
>        return true;
>      else if (A.Name == B.Name && A.Size < B.Size)
>        return true;
> -    else if (A.Name == B.Name && A.Size == B.Size && A.Address <
> B.Address)
> -      return true;
>      else
> -      return false;
> +      return A.Name == B.Name && A.Size == B.Size && A.Address <
> B.Address;
>    } else {
>      if (A.Name > B.Name)
>        return true;
>      else if (A.Name == B.Name && A.Size > B.Size)
>        return true;
> -    else if (A.Name == B.Name && A.Size == B.Size && A.Address >
> B.Address)
> -      return true;
>      else
> -      return false;
> +      return A.Name == B.Name && A.Size == B.Size && A.Address >
> B.Address;
>    }
>  }
>
> @@ -263,10 +251,8 @@
>      return true;
>    else if (isa<ELF32BEObjectFile>(Obj))
>      return false;
> -  else if (isa<ELF64BEObjectFile>(Obj))
> -    return true;
>    else
> -    return false;
> +    return isa<ELF64BEObjectFile>(Obj);
>  }
>
>  static StringRef CurrentFilename;
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150322/3110c853/attachment.html>


More information about the llvm-commits mailing list