[PATCH] D87272: [lld] Buffer writes when composing a single diagnostic

[Fangrui Song via Phabricator via llvm-commits]

MaskRay updated this revision to Diff 290759.
MaskRay added a comment.

Adopt jhenderson's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87272

Files:
  lld/Common/ErrorHandler.cpp
  lld/include/lld/Common/ErrorHandler.h
  llvm/include/llvm/Support/raw_ostream.h


Index: llvm/include/llvm/Support/raw_ostream.h
===================================================================
--- llvm/include/llvm/Support/raw_ostream.h
+++ llvm/include/llvm/Support/raw_ostream.h
@@ -301,6 +301,8 @@
   // changeColor() has no effect until enable_colors(true) is called.
   virtual void enable_colors(bool enable) { ColorEnabled = enable; }
 
+  bool colors_enabled() const { return ColorEnabled; }
+
   /// Tie this stream to the specified stream. Replaces any existing tied-to
   /// stream. Specifying a nullptr unties the stream.
   void tie(raw_ostream *TieTo) { TiedStream = TieTo; }
Index: lld/include/lld/Common/ErrorHandler.h
===================================================================
--- lld/include/lld/Common/ErrorHandler.h
+++ lld/include/lld/Common/ErrorHandler.h
@@ -112,6 +112,8 @@
   using Colors = raw_ostream::Colors;
 
   std::string getLocation(const Twine &msg);
+  void reportDiagnostic(StringRef location, Colors c, StringRef diagKind,
+                        const Twine &msg);
 };
 
 /// Returns the default error handler.
Index: lld/Common/ErrorHandler.cpp
===================================================================
--- lld/Common/ErrorHandler.cpp
+++ lld/Common/ErrorHandler.cpp
@@ -152,11 +152,27 @@
   return std::string(logName);
 }
 
+void ErrorHandler::reportDiagnostic(StringRef location, Colors c,
+                                    StringRef diagKind, const Twine &msg) {
+  SmallString<256> buf;
+  raw_svector_ostream os(buf);
+  os << sep << location << ": ";
+  if (!diagKind.empty()) {
+    if (lld::errs().colors_enabled()) {
+      os.enable_colors(true);
+      os << c << diagKind << ": " << Colors::RESET;
+    } else {
+      os << diagKind << ": ";
+    }
+  }
+  os << msg << '\n';
+  std::lock_guard<std::mutex> lock(mu);
+  lld::errs() << buf;
+}
+
 void ErrorHandler::log(const Twine &msg) {
-  if (!verbose)
-    return;
-  std::lock_guard<std::mutex> lock(mu);
-  lld::errs() << logName << ": " << msg << "\n";
+  if (verbose)
+    reportDiagnostic(logName, Colors::RESET, "", msg);
 }
 
 void ErrorHandler::message(const Twine &msg) {
@@ -171,9 +187,7 @@
     return;
   }
 
-  std::lock_guard<std::mutex> lock(mu);
-  lld::errs() << sep << getLocation(msg) << ": " << Colors::MAGENTA
-              << "warning: " << Colors::RESET << msg << "\n";
+  reportDiagnostic(getLocation(msg), Colors::MAGENTA, "warning", msg);
   sep = getSeparator(msg);
 }
 
@@ -196,15 +210,10 @@
 
   bool exit = false;
   {
-    std::lock_guard<std::mutex> lock(mu);
-
     if (errorLimit == 0 || errorCount < errorLimit) {
-      lld::errs() << sep << getLocation(msg) << ": " << Colors::RED
-                  << "error: " << Colors::RESET << msg << "\n";
+      reportDiagnostic(getLocation(msg), Colors::RED, "error", msg);
     } else if (errorCount == errorLimit) {
-      lld::errs() << sep << getLocation(msg) << ": " << Colors::RED
-                  << "error: " << Colors::RESET << errorLimitExceededMsg
-                  << "\n";
+      reportDiagnostic(logName, Colors::RED, "error", errorLimitExceededMsg);
       exit = exitEarly;
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87272.290759.patch
Type: text/x-patch
Size: 3135 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200909/9ae69761/attachment.bin>