[PATCH] D64165: [llvm-objdump] Keep warning for --disassemble-functions in correct order.

Yuanfang Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 8 08:44:33 PDT 2019


ychen marked 4 inline comments as done.
ychen added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:384
 
-void warn(Twine Message) {
-  WithColor::warning(errs(), ToolName) << Message << "\n";
+static void warn(Twine Message, StringRef Prefix = "") {
+  // Output order between errs() and outs() matters especially for archive
----------------
MaskRay wrote:
> Why was `StringRef Prefix` added? It does not seem to be used.
It was intended for the patch for PR35351. I've removed it. 


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:389
+  WithColor::warning(errs(), Prefix) << Message << "\n";
+  errs().flush();
 }
----------------
MaskRay wrote:
> This is probably redundant. stderr is by default unbuffered. If the user makes it buffered, they should be prepared to see out-of-order output.
Agree that it seems redundant but it makes explicit that stderr should be flushed there.  Three other places in the same file use it too. I could remove them all or keep them all otherwise it is really confusing and inconsistent. Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64165





More information about the llvm-commits mailing list