[lld] r299511 - Simplify. NFC.

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 5 08:38:38 PDT 2017


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; });
 }

 uint32_t LinkerScript::getFiller(StringRef Name) {


-- 
Davide

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


More information about the llvm-commits mailing list