[PATCH] D68906: [llvm-size] Tidy up error messages

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 12 03:29:34 PDT 2019


grimar added inline comments.


================
Comment at: llvm/tools/llvm-size/llvm-size.cpp:110
 
-static bool error(Twine Message) {
+static bool error(StringRef File, Twine Message) {
   HadError = true;
----------------
MaskRay wrote:
> I'd prefer `error(Twine Message, StringRef File)`. Note the error below places the error (for error message) at the first argument position.
Yeah. But it should be `const Twine &`
("Twines should only be used accepted as const references in arguments"
https://llvm.org/doxygen/classllvm_1_1Twine.html#details)

Also, why does it return `bool`? Can it be `void`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68906





More information about the llvm-commits mailing list