[PATCH] [dsymutil] Implement the BinaryHolder object and gain archive support.

Frederic Riss friss at apple.com
Mon Jan 5 10:09:15 PST 2015


================
Comment at: tools/dsymutil/BinaryHolder.cpp:58
@@ +57,3 @@
+  StringRef CurArchiveName = CurrentArchive->getFileName();
+  if (Filename.size() <= CurArchiveName.size() ||
+      !Filename.startswith(CurArchiveName) ||
----------------
samsonov wrote:
> will this work?
>   Filename.startswith(Twine(CurArchiveName, "(").str())
This would certainly work, but it adds a string copy. Not that this will show up on profiles though... OK.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:106
@@ +105,3 @@
+      object::ObjectFile::createObjectFile(*ErrOrMemBufferRef);
+  if (auto Err = ErrOrMemBufferRef.getError())
+    return Err;
----------------
samsonov wrote:
> s/ErrOrMemBufferRef/ErrOrObjectFile
sure. Leftover from an old implementation.

================
Comment at: tools/dsymutil/BinaryHolder.h:75
@@ +74,3 @@
+  /// the BinaryHolder.
+  ErrorOr<const object::ObjectFile &> GetObjectFile(StringRef Filename);
+
----------------
samsonov wrote:
> Does it need to be a public method?
Not in this version of the patch, but I don't see a compelling reason to make it private. Consumers that don't care about the object file type should call this method (the Dwarf linker is one of these consumers). Any strong objection to leaving it public?

http://reviews.llvm.org/D6690

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list