[PATCH] D48413: [llvm-objcopy] Add initial support for static libraries

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/Object.cpp:39
+  handleAllErrors(BufferOrErr.takeError(), [this](const ErrorInfoBase &) {
+    error("failed to open " + getName());
+  });
----------------
jhenderson wrote:
> alexshap wrote:
> > alexshap wrote:
> > > jhenderson wrote:
> > > > This should use the error info's message. Otherwise, the user won't be able to know what is wrong. I think this is probably also one that is easy to test, by specifying a location in a non-existent directory?
> > > the thing is that I moved this from the previous version (before my patch) without any of may changes, but okay, I'll fix it
> > typo: may -> my
> Sorry, I should have been clearer. I meant, I'd use the error info's message prefixed with the original "failed to open <file>", so:
> 
> `failed to open foo.a: no such file or directory`
> 
> or whatever.
> 
> Actually, this just occurred to me - is the "no such file or directory" a platform independent error message? Otherwise we won't be able to test as easily (in that case I'd keep the extra information printing, but test only up to the colon).
yeah, No such file or directory is OS-dependent, i.e. see ./test/Object/ar-error.test


Repository:
  rL LLVM

https://reviews.llvm.org/D48413