[PATCH] D96310: [llvm-objcopy] Delete --build-id-link-{dir,input,output}

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 12 11:23:52 PST 2021


MaskRay added inline comments.


================
Comment at: llvm/docs/ReleaseNotes.rst:128
+* ``--build-id-link-{dir,input,output}`` have been deleted.
+  (`D96310 <https://reviews.llvm.org/D96310>`_)
+
----------------
jhenderson wrote:
> Dumb question maybe, but what's the trailing `_` for?
The Embed URI syntax requires it. https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#embedded-uris-and-aliases


================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.h:446
 
+  ArrayRef<uint8_t> Contents;
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > MaskRay wrote:
> > > jhenderson wrote:
> > > > This change seems unrelated?
> > > This is natural after the last public user of Contents is deleted.
> > > 
> > > It does not seem necessary to defer it to a subsequent clean-up.
> > I'd agree with that, except I can't see where Contents was previously being used?
> Ping this? I have no issue with it being moved, but if it wasn't being used publicly before this patch, it probably should be a separate change.
The original patch adding the featured move the variable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96310/new/

https://reviews.llvm.org/D96310



More information about the llvm-commits mailing list