<div dir="ltr"><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">Hi, </span></font><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">many thanks for looking at the diff.</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">(i started working on this because this interface change broke some out-of-tree code, but that's expected and not a big issue, i just wanted to clean it up a bit). </span></font></div><div><span style="color:rgb(51,51,51);font-family:"normal arial",sans-serif;font-size:16px"><br></span></div><div><span style="color:rgb(51,51,51);font-family:"normal arial",sans-serif;font-size:16px">I assume I might be missing smth, </span></div><div><span style="color:rgb(51,51,51);font-family:"normal arial",sans-serif;font-size:16px">however my diff doesn't change the static method </span><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px"> </span></font></div><div><span style="font-size:16px;color:rgb(51,51,51);font-family:"normal arial",sans-serif">/// For use examining a trie not in a MachOObjectFile.</span><br></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">static iterator_range<export_iterator> exports(Error &Err,</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">                                                 ArrayRef<uint8_t> Trie,</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">                                                 const MachOObjectFile *O =</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">                                                                      nullptr);</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px"><br></span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">It changes only the regular method  </span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">/// For use iterating over all exported symbols.</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">iterator_range<export_iterator> exports(Error &Err, </span></font><span style="color:rgb(51,51,51);font-family:"normal arial",sans-serif;font-size:16px">const MachOObjectFile *O</span><span style="font-size:16px;color:rgb(51,51,51);font-family:"normal arial",sans-serif">) const;</span></div><div><span style="font-size:16px;color:rgb(51,51,51);font-family:"normal arial",sans-serif">because I didn't like syntax Obj->exports(Err, Obj).</span></div><div><span style="font-size:16px;color:rgb(51,51,51);font-family:"normal arial",sans-serif">LLD builds successfully with my patch - so I am wondering if this change looks reasonable/safe to you.</span></div><div><span style="font-size:16px;color:rgb(51,51,51);font-family:"normal arial",sans-serif"> </span><br></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">Thanks,</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px">Alex Shaposhnikov</span></font></div><div><font color="#333333" face="normal arial, sans-serif"><span style="font-size:16px"><br></span></font></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 28, 2017 at 10:30 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Fri, Jul 28, 2017 at 10:24 AM Kevin Enderby <<a href="mailto:enderby@apple.com" target="_blank">enderby@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Jul 27, 2017, at 9:50 PM, Saleem Abdulrasool via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:</div><br class="m_5464274064247327764m_1946500766084757489Apple-interchange-newline"><div><div>compnerd added a comment.<br><br>Does anyone use the overload with the `O` for `exports` with `nullptr` instead of `this`?  If not, we could just inline `this` throughout.<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div>This will break lld as it needs the default nullptr.  See <span style="font-family:'Helvetica Neue'">r308691.</span></div><div><span style="font-family:'Helvetica Neue'"><br></span></div><div><span style="font-family:'Helvetica Neue'">The reason that O was added was so that this check from r</span>308690 could be added.</div></div></blockquote></span><div><br>When O is specified is it always == this, though? That seems to be implied by the original post.<br><br>If that's the case, then the ability to pass null is really only passing the on/off state for the diagnostic? Is that necessary, or would it be good to just always produce the Error, even in lld?<br> </div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span style="font-family:'Helvetica Neue'"><br></span></div><div>+      if (O != nullptr) {<br>+        if (State.Other > O->getLibraryCount()) {<br>+          *E = malformedError("bad library ordinal: " + Twine((int)State.Other)<br>+               + " (max " + Twine((int)O->getLibraryCount(<wbr>)) + ") in export "<br>+               "trie data at node: 0x" + utohexstr(offset));<br>+          moveToEnd();<br>+          return;<br>+        }</div><div><br></div><div>This is needed for the test case:</div><div><br></div><div>+RUN: not llvm-objdump -macho -exports-trie %p/Inputs/macho-trie-bad-<wbr>library-ordinal 2>&1 | FileCheck -check-prefix BAD_LIBRARY_ORDINAL %s <br>+BAD_LIBRARY_ORDINAL: macho-trie-bad-library-<wbr>ordinal': truncated or malformed object (bad library ordinal: 69 (max 3) in export trie data at node: 0x33)</div></div><div style="word-wrap:break-word"><div><font face="Helvetica Neue"><br></font><blockquote type="cite"><div><div><br><br><br>================<br>Comment at: tools/llvm-nm/llvm-nm.cpp:1230<br>       Error Err = Error::success();<br>-      for (const llvm::object::ExportEntry &Entry : MachO->exports(Err,<br>-                               <wbr>                              <wbr>      MachO)) {<br>+      for (const llvm::object::ExportEntry &Entry : MachO->exports(Err)) {<br>         bool found = false;<br>----------------<br>I think that using `auto` here instead of `llvm::object:ExportEntry` is better for readability.<br><br><br>================<br>Comment at: tools/llvm-objdump/MachODump.<wbr>cpp:9406<br>   Error Err = Error::success();<br>-  for (const llvm::object::ExportEntry &Entry : Obj->exports(Err, Obj)) {<br>+  for (const llvm::object::ExportEntry &Entry : Obj->exports(Err)) {<br>     uint64_t Flags = Entry.flags();<br>----------------<br>Similar.<br><br><br>Repository:<br>  rL LLVM<br><br><a href="https://reviews.llvm.org/D35961" target="_blank">https://reviews.llvm.org/<wbr>D35961</a><br><br><br><br></div></div></blockquote></div><br></div></blockquote></div></div></div></div>
</blockquote></div><br></div>