[lld] r299511 - Simplify. NFC.

[Davide Italiano via llvm-commits]

On Wed, Apr 5, 2017 at 12:13 PM, Rui Ueyama <ruiu at google.com> wrote:
> 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.
>

Either way, I don't care :) I just found the use of `find_if == end()`
very weird


-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare