[lld] r299511 - Simplify. NFC.

[Rui Ueyama via llvm-commits]

On Wed, Apr 5, 2017 at 8:38 AM, Davide Italiano <davide at freebsd.org> wrote:

> On Tue, Apr 4, 2017 at 10:55 PM, Rui Ueyama <ruiu at google.com> wrote:
> > On Tue, Apr 4, 2017 at 10:23 PM, Davide Italiano <davide at freebsd.org>
> wrote:
> >>
> >> On Tue, Apr 4, 2017 at 10:06 PM, Rui Ueyama via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >> > Author: ruiu
> >> > Date: Wed Apr  5 00:06:17 2017
> >> > New Revision: 299511
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=299511&view=rev
> >> > Log:
> >> > Simplify. NFC.
> >> >
> >> > A for-loop is more boring than a find_if, but I think this is easier
> to
> >> > read.
> >> >
> >> > Modified:
> >> >     lld/trunk/ELF/LinkerScript.cpp
> >> >
> >> > Modified: lld/trunk/ELF/LinkerScript.cpp
> >> > URL:
> >> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> LinkerScript.cpp?rev=299511&r1=299510&r2=299511&view=diff
> >> >
> >> > ============================================================
> ==================
> >> > --- lld/trunk/ELF/LinkerScript.cpp (original)
> >> > +++ lld/trunk/ELF/LinkerScript.cpp Wed Apr  5 00:06:17 2017
> >> > @@ -923,10 +923,12 @@ std::vector<PhdrEntry> LinkerScript::cre
> >> >  bool LinkerScript::ignoreInterpSection() {
> >> >    // Ignore .interp section in case we have PHDRS specification
> >> >    // and PT_INTERP isn't listed.
> >> > -  return !Opt.PhdrsCommands.empty() &&
> >> > -         llvm::find_if(Opt.PhdrsCommands, [](const PhdrsCommand
> &Cmd) {
> >> > -           return Cmd.Type == PT_INTERP;
> >> > -         }) == Opt.PhdrsCommands.end();
> >> > +  if (Opt.PhdrsCommands.empty())
> >> > +    return false;
> >> > +  for (PhdrsCommand &Cmd : Opt.PhdrsCommands)
> >> > +    if (Cmd.Type == PT_INTERP)
> >> > +      return false;
> >> > +  return true;
> >> >  }
> >> >
> >>
> >> Maybe the `find_if` construct was too obscure, but now that you split
> >> the first `if` out, you may consider replacing the phdrs command loop
> >> with `all_of`, which I think it's an improvement in readability. I
> >> don't feel strong about it tho, up to you.
> >
> >
> > You still need to use `find_if` instead of `all_of`, no?
>
> I don't think you need `find_if` (unless I'm reading the code wrong,
> which could be possible). I originally thought you needed to check
> universality while you need non-existence, so none_of is what you
>  want.
>
> diff --git a/ELF/LinkerScript.cpp b/ELF/LinkerScript.cpp
> index 924a0f3..22073c8 100644
> --- a/ELF/LinkerScript.cpp
> +++ b/ELF/LinkerScript.cpp
> @@ -859,10 +859,8 @@ bool LinkerScript::ignoreInterpSection() {
>    // and PT_INTERP isn't listed.
>    if (Opt.PhdrsCommands.empty())
>      return false;
> -  for (PhdrsCommand &Cmd : Opt.PhdrsCommands)
> -    if (Cmd.Type == PT_INTERP)
> -      return false;
> -  return true;
> +  return llvm::none_of(Opt.PhdrsCommands,
> +                       [](PhdrsCommand &Cmd) { return Cmd.Type ==
> PT_INTERP; });
>  }
>

Ah. But I think I'd still prefer the for-loop version, although it's not a
strong preference.

 uint32_t LinkerScript::getFiller(StringRef Name) {
>
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170405/a4308ce8/attachment.html>