[PATCH] D39713: [llvm-objcopy] Change -O binary to respect section removal and behave like GNU objcopy

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

Three more minor points, but LGTM with the suggested fixes.



================
Comment at: tools/llvm-objcopy/Object.cpp:708
+// This function assumes that Segments have been sorted by OrderSegments. This
+// function returns an Offset one past the end of the last segment.
+static uint64_t LayoutSegments(std::vector<Segment *> &Segments,
----------------
jhenderson wrote:
> jhenderson wrote:
> > I don't think you need to repeat "This function" at the start of the second sentence. Simply "It", or combining the two sentences would suffice.
> Okay, my bad. That didn't come out quite as I meant/felt it would, and it now feels a little unnatural. As I believe that the LLVM standard is for full sentence, that first sentence needs to be either "This function finds a..." or "Find a...". Also, you've got "of list" when you need "list of".
> 
> Depending on which of the two suggestions you use for the first sentence, you should change the start of the second and third sentences accordingly. If the first approach is used, don't change the second sentence and merge the two: "It assumes ... OrderSegments, and returns ...". If using the second approach, you need to put "This function" instead of the first "It". Optionally, you can then fold the two sentences together as in the previous case.
 OrderSegments and It returns -> OrderSegments and returns


================
Comment at: tools/llvm-objcopy/Object.cpp:736
+
+// This function finds a consistent layout of a list of sections. It assumes
+// that the ->ParentSegment of each section has already been laid out. The
----------------
layout of -> layout for


================
Comment at: tools/llvm-objcopy/Object.cpp:869
+  // segments in OrderedSegments. If there were duplicates then LayoutSegments
+  // would do very stranges things.
+  auto End =
----------------
stranges -> strange


Repository:
  rL LLVM

https://reviews.llvm.org/D39713