[PATCH] D58316: [llvm-objcopy][NFC] More error cleanup

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 20 02:16:28 PST 2019


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

LGTM.



================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:224
+    return createStringError(errc::invalid_argument,
+                             "Invalid architecture: '%s'.", Arch.str().c_str());
   return Iter->getValue();
----------------
rupprecht wrote:
> jhenderson wrote:
> > You've added an extra '.' at the end of this error message. I think our standard is (usually) to not have them. (We might want to tidy up the other messages that do have them already).
> llvm-objcopy's error() automatically adds a "." when printing the error message, but printing w/ the default handler does not automatically add it, and we assert it ends with "." in some tests.
> 
> I don't have a preference, the added "." was to match that behavior. I've gone ahead and fixed all the tests that were checking for a trailing "." instead and reverted these bits. I can do other files in a separate change.
I could have sworn I double-checked that! Either way, moving away from trailing full-stops is probably the right thing to do, as I think the majority of error messages from other tools do not use them (which I think is wrong, but we should be consistent). Perhaps @jakehehrlich has got something to say on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58316





More information about the llvm-commits mailing list