[SourceMgr][FileCheck] Obey -color by extending WithColor

[jdenny]

Summary

While this change specifically targets FileCheck, it affects any tool
using the same SourceMgr facilities.

Previously, -color was documented in FileCheck's -help output, but
-color had no effect. Now, -color obeys its documentation: it forces
colors to be used in FileCheck diagnostics even when stderr is not a
terminal.

-color is especially helpful when combined with FileCheck's -v, which
can produce a long series of diagnostics that you might wish to pipe
to a pager, such as less -R. The WithColor extensions here will also
help to clean up color usage in FileCheck's annotated dump of input,
which is proposed in D52999.

[JDevlieghere]

LGTM.

[jdenny]

In D53419#1269236, @JDevlieghere wrote:

LGTM.

Thanks. Will commit in a few days when I'm available to watch the buildbots.

[jdenny]

The patch was reverted because the test it introduces fails on some Windows bots where color isn't supported as expected.

I've added a small change: at the start of FileCheck's main, call llvm::sys::Process::UseANSIEscapeCodes(true) unconditionally. This is a no-op for Linux, but it fixes the test for my Windows 10 install, and it seems to make FileCheck colors behave ok in my brief cmd.exe experiments.

However, I'm not a Windows developer. I see that each of clang and lldb handles Windows colors in its own unique and more complex way, which I haven't fully grasped.

I've added some reviewers who, according to git blame, have worked with Windows colors for LLVM before. Before I commit again, could someone please comment as to whether this change looks ok for FileCheck under Windows?

Thanks.

[zturner]

Seems ok to me, my only concern is that something could fail on Windows 8 or Windows 7, but I don't have a system to test that on.

[jdenny]

In D53419#1273329, @zturner wrote:

Seems ok to me, my only concern is that something could fail on Windows 8 or Windows 7, but I don't have a system to test that on.

Thanks. I'll try to push tomorrow when I can watch the bots more carefully. Hopefully any problems will be caught there.

llvm/trunk/docs/CommandGuide/FileCheck.rst

 
 .. option:: --allow-deprecated-dag-overlap
 
   Enable overlapping among matches in a group of consecutive ``CHECK-DAG:``
   directives.  This option is deprecated and is only provided for convenience
   as old tests are migrated to the new non-overlapping ``CHECK-DAG:``
   implementation.
 
+.. option:: --color
+
+  Use colors in output (autodetected by default).
+
 EXIT STATUS
 -----------
 
 If :program:`FileCheck` verifies that the file matches the expected contents,
 it exits with 0.  Otherwise, if not, or if an error occurs, it will exit with a
 non-zero value.
 
 TUTORIAL

llvm/trunk/include/llvm/Support/WithColor.h

enum class HighlightColor {
   Address,
   String,
   Tag,
   Attribute,
   Enumerator,
   Macro,
   Error,
   Warning,
-  Note
+  Note,
+  Remark
 };
 
 /// An RAII object that temporarily switches an output stream to a specific
 /// color.
 class WithColor {
   raw_ostream &OS;
-  /// Determine whether colors should be displayed.
-  bool colorsEnabled(raw_ostream &OS);
+  bool DisableColors;
 
 public:
   /// To be used like this: WithColor(OS, HighlightColor::String) << "text";
-  WithColor(raw_ostream &OS, HighlightColor S);
+  /// @param OS The output stream
+  /// @param S Symbolic name for syntax element to color
+  /// @param DisableColors Whether to ignore color changes regardless of -color
+  /// and support in OS
+  WithColor(raw_ostream &OS, HighlightColor S, bool DisableColors = false);
+  /// To be used like this: WithColor(OS, raw_ostream::Black) << "text";
+  /// @param OS The output stream
+  /// @param Color ANSI color to use, the special SAVEDCOLOR can be used to
+  /// change only the bold attribute, and keep colors untouched
+  /// @param Bold Bold/brighter text, default false
+  /// @param BG If true, change the background, default: change foreground
+  /// @param DisableColors Whether to ignore color changes regardless of -color
+  /// and support in OS
+  WithColor(raw_ostream &OS,
+            raw_ostream::Colors Color = raw_ostream::SAVEDCOLOR,
+            bool Bold = false, bool BG = false, bool DisableColors = false)
+      : OS(OS), DisableColors(DisableColors) {
+    changeColor(Color, Bold, BG);
+  }
   ~WithColor();
 
   raw_ostream &get() { return OS; }
   operator raw_ostream &() { return OS; }
+  template <typename T> WithColor &operator<<(T &O) {
+    OS << O;
+    return *this;
+  }
+  template <typename T> WithColor &operator<<(const T &O) {
+    OS << O;
+    return *this;
+  }
 
   /// Convenience method for printing "error: " to stderr.
   static raw_ostream &error();
   /// Convenience method for printing "warning: " to stderr.
   static raw_ostream &warning();
   /// Convenience method for printing "note: " to stderr.
   static raw_ostream &note();
+  /// Convenience method for printing "remark: " to stderr.
+  static raw_ostream &remark();
 
   /// Convenience method for printing "error: " to the given stream.
-  static raw_ostream &error(raw_ostream &OS, StringRef Prefix = "");
+  static raw_ostream &error(raw_ostream &OS, StringRef Prefix = "",
+                            bool DisableColors = false);
   /// Convenience method for printing "warning: " to the given stream.
-  static raw_ostream &warning(raw_ostream &OS, StringRef Prefix = "");
+  static raw_ostream &warning(raw_ostream &OS, StringRef Prefix = "",
+                              bool DisableColors = false);
   /// Convenience method for printing "note: " to the given stream.
-  static raw_ostream &note(raw_ostream &OS, StringRef Prefix = "");
+  static raw_ostream &note(raw_ostream &OS, StringRef Prefix = "",
+                           bool DisableColors = false);
+  /// Convenience method for printing "remark: " to the given stream.
+  static raw_ostream &remark(raw_ostream &OS, StringRef Prefix = "",
+                             bool DisableColors = false);
+
+  /// Determine whether colors are displayed.
+  bool colorsEnabled();
+
+  /// Change the color of text that will be output from this point forward.
+  /// @param Color ANSI color to use, the special SAVEDCOLOR can be used to
+  /// change only the bold attribute, and keep colors untouched
+  /// @param Bold Bold/brighter text, default false
+  /// @param BG If true, change the background, default: change foreground
+  WithColor &changeColor(raw_ostream::Colors Color, bool Bold = false,
+                         bool BG = false);
+
+  /// Reset the colors to terminal defaults. Call this when you are done
+  /// outputting colored text, or before program exit.
+  WithColor &resetColor();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_DEBUGINFO_WITHCOLOR_H

llvm/trunk/lib/Support/SourceMgr.cpp

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Locale.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SMLoc.h"
+#include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <limits>
 #include <memory>
 #include <string>
 #include <utility>
static void printSourceLine(raw_ostream &S, StringRef LineContents) {
   }
   S << '\n';
 }
 
 static bool isNonASCII(char c) {
   return c & 0x80;
 }
 
-void SMDiagnostic::print(const char *ProgName, raw_ostream &S, bool ShowColors,
-                         bool ShowKindLabel) const {
-  // Display colors only if OS supports colors.
-  ShowColors &= S.has_colors();
-
-  if (ShowColors)
-    S.changeColor(raw_ostream::SAVEDCOLOR, true);
+void SMDiagnostic::print(const char *ProgName, raw_ostream &OS,
+                         bool ShowColors, bool ShowKindLabel) const {
+  {
+    WithColor S(OS, raw_ostream::SAVEDCOLOR, true, false, !ShowColors);
 
     if (ProgName && ProgName[0])
       S << ProgName << ": ";
 
     if (!Filename.empty()) {
       if (Filename == "-")
         S << "<stdin>";
       else
         S << Filename;
 
       if (LineNo != -1) {
         S << ':' << LineNo;
         if (ColumnNo != -1)
-        S << ':' << (ColumnNo+1);
+          S << ':' << (ColumnNo + 1);
       }
       S << ": ";
     }
+  }
 
   if (ShowKindLabel) {
     switch (Kind) {
     case SourceMgr::DK_Error:
-      if (ShowColors)
-        S.changeColor(raw_ostream::RED, true);
-      S << "error: ";
+      WithColor::error(OS, "", !ShowColors);
       break;
     case SourceMgr::DK_Warning:
-      if (ShowColors)
-        S.changeColor(raw_ostream::MAGENTA, true);
-      S << "warning: ";
+      WithColor::warning(OS, "", !ShowColors);
       break;
     case SourceMgr::DK_Note:
-      if (ShowColors)
-        S.changeColor(raw_ostream::BLACK, true);
-      S << "note: ";
+      WithColor::note(OS, "", !ShowColors);
       break;
     case SourceMgr::DK_Remark:
-      if (ShowColors)
-        S.changeColor(raw_ostream::BLUE, true);
-      S << "remark: ";
+      WithColor::remark(OS, "", !ShowColors);
       break;
     }
-
-    if (ShowColors) {
-      S.resetColor();
-      S.changeColor(raw_ostream::SAVEDCOLOR, true);
-    }
   }
 
-  S << Message << '\n';
-
-  if (ShowColors)
-    S.resetColor();
+  WithColor(OS, raw_ostream::SAVEDCOLOR, true, false, !ShowColors)
+      << Message << '\n';
 
   if (LineNo == -1 || ColumnNo == -1)
     return;
 
   // FIXME: If there are multibyte or multi-column characters in the source, all
   // our ranges will be wrong. To do this properly, we'll need a byte-to-column
   // map like Clang's TextDiagnostic. For now, we'll just handle tabs by
   // expanding them later, and bail out rather than show incorrect ranges and
   // misaligned fixits for any other odd characters.
   if (find_if(LineContents, isNonASCII) != LineContents.end()) {
-    printSourceLine(S, LineContents);
+    printSourceLine(OS, LineContents);
     return;
   }
   size_t NumColumns = LineContents.size();
 
   // Build the line with the caret and ranges.
   std::string CaretLine(NumColumns+1, ' ');
 
   // Expand any ranges.
void SMDiagnostic::print(const char *ProgName, raw_ostream &OS,
   else
     CaretLine[NumColumns] = '^';
 
   // ... and remove trailing whitespace so the output doesn't wrap for it.  We
   // know that the line isn't completely empty because it has the caret in it at
   // least.
   CaretLine.erase(CaretLine.find_last_not_of(' ')+1);
 
-  printSourceLine(S, LineContents);
+  printSourceLine(OS, LineContents);
 
-  if (ShowColors)
-    S.changeColor(raw_ostream::GREEN, true);
+  {
+    WithColor S(OS, raw_ostream::GREEN, true, false, !ShowColors);
 
     // Print out the caret line, matching tabs in the source line.
     for (unsigned i = 0, e = CaretLine.size(), OutCol = 0; i != e; ++i) {
       if (i >= LineContents.size() || LineContents[i] != '\t') {
         S << CaretLine[i];
         ++OutCol;
         continue;
       }
 
       // Okay, we have a tab.  Insert the appropriate number of characters.
       do {
         S << CaretLine[i];
         ++OutCol;
       } while ((OutCol % TabStop) != 0);
     }
     S << '\n';
-
-  if (ShowColors)
-    S.resetColor();
+  }
 
   // Print out the replacement line, matching tabs in the source line.
   if (FixItInsertionLine.empty())
     return;
 
   for (size_t i = 0, e = FixItInsertionLine.size(), OutCol = 0; i < e; ++i) {
     if (i >= LineContents.size() || LineContents[i] != '\t') {
-      S << FixItInsertionLine[i];
+      OS << FixItInsertionLine[i];
       ++OutCol;
       continue;
     }
 
     // Okay, we have a tab.  Insert the appropriate number of characters.
     do {
-      S << FixItInsertionLine[i];
+      OS << FixItInsertionLine[i];
       // FIXME: This is trying not to break up replacements, but then to re-sync
       // with the tabs between replacements. This will fail, though, if two
       // fix-it replacements are exactly adjacent, or if a fix-it contains a
       // space. Really we should be precomputing column widths, which we'll
       // need anyway for multibyte chars.
       if (FixItInsertionLine[i] != ' ')
         ++i;
       ++OutCol;
     } while (((OutCol % TabStop) != 0) && i != e);
   }
-  S << '\n';
+  OS << '\n';
 }

llvm/trunk/lib/Support/WithColor.cpp

 
 cl::OptionCategory llvm::ColorCategory("Color Options");
 
 static cl::opt<cl::boolOrDefault>
     UseColor("color", cl::cat(ColorCategory),
              cl::desc("Use colors in output (default=autodetect)"),
              cl::init(cl::BOU_UNSET));
 
-bool WithColor::colorsEnabled(raw_ostream &OS) {
-  if (UseColor == cl::BOU_UNSET)
-    return OS.has_colors();
-  return UseColor == cl::BOU_TRUE;
-}
-
-WithColor::WithColor(raw_ostream &OS, HighlightColor Color) : OS(OS) {
+WithColor::WithColor(raw_ostream &OS, HighlightColor Color, bool DisableColors)
+    : OS(OS), DisableColors(DisableColors) {
   // Detect color from terminal type unless the user passed the --color option.
-  if (colorsEnabled(OS)) {
+  if (colorsEnabled()) {
     switch (Color) {
     case HighlightColor::Address:
       OS.changeColor(raw_ostream::YELLOW);
       break;
     case HighlightColor::String:
       OS.changeColor(raw_ostream::GREEN);
       break;
     case HighlightColor::Tag:
case HighlightColor::Error:
       OS.changeColor(raw_ostream::RED, true);
       break;
     case HighlightColor::Warning:
       OS.changeColor(raw_ostream::MAGENTA, true);
       break;
     case HighlightColor::Note:
       OS.changeColor(raw_ostream::BLACK, true);
       break;
+    case HighlightColor::Remark:
+      OS.changeColor(raw_ostream::BLUE, true);
+      break;
     }
   }
 }
 
 raw_ostream &WithColor::error() { return error(errs()); }
 
 raw_ostream &WithColor::warning() { return warning(errs()); }
 
 raw_ostream &WithColor::note() { return note(errs()); }
 
-raw_ostream &WithColor::error(raw_ostream &OS, StringRef Prefix) {
+raw_ostream &WithColor::remark() { return remark(errs()); }
+
+raw_ostream &WithColor::error(raw_ostream &OS, StringRef Prefix,
+                              bool DisableColors) {
   if (!Prefix.empty())
     OS << Prefix << ": ";
-  return WithColor(OS, HighlightColor::Error).get() << "error: ";
+  return WithColor(OS, HighlightColor::Error, DisableColors).get()
+         << "error: ";
 }
 
-raw_ostream &WithColor::warning(raw_ostream &OS, StringRef Prefix) {
+raw_ostream &WithColor::warning(raw_ostream &OS, StringRef Prefix,
+                                bool DisableColors) {
   if (!Prefix.empty())
     OS << Prefix << ": ";
-  return WithColor(OS, HighlightColor::Warning).get() << "warning: ";
+  return WithColor(OS, HighlightColor::Warning, DisableColors).get()
+         << "warning: ";
 }
 
-raw_ostream &WithColor::note(raw_ostream &OS, StringRef Prefix) {
+raw_ostream &WithColor::note(raw_ostream &OS, StringRef Prefix,
+                             bool DisableColors) {
   if (!Prefix.empty())
     OS << Prefix << ": ";
-  return WithColor(OS, HighlightColor::Note).get() << "note: ";
+  return WithColor(OS, HighlightColor::Note, DisableColors).get() << "note: ";
 }
 
-WithColor::~WithColor() {
-  if (colorsEnabled(OS))
+raw_ostream &WithColor::remark(raw_ostream &OS, StringRef Prefix,
+                               bool DisableColors) {
+  if (!Prefix.empty())
+    OS << Prefix << ": ";
+  return WithColor(OS, HighlightColor::Remark, DisableColors).get()
+         << "remark: ";
+}
+
+bool WithColor::colorsEnabled() {
+  if (DisableColors)
+    return false;
+  if (UseColor == cl::BOU_UNSET)
+    return OS.has_colors();
+  return UseColor == cl::BOU_TRUE;
+}
+
+WithColor &WithColor::changeColor(raw_ostream::Colors Color, bool Bold,
+                                  bool BG) {
+  if (colorsEnabled())
+    OS.changeColor(Color, Bold, BG);
+  return *this;
+}
+
+WithColor &WithColor::resetColor() {
+  if (colorsEnabled())
     OS.resetColor();
+  return *this;
 }
+
+WithColor::~WithColor() { resetColor(); }

llvm/trunk/test/FileCheck/opt-color.txt

+; Create a case that produces a simple diagnostic.
+; RUN: echo foo > %t.in
+; CHECK: bar
+
+; Run without and with -color.  In the former case, FileCheck should suppress
+; color in its diagnostics because stderr is a file.
+; RUN: not FileCheck %s < %t.in 2> %t.no-color
+; RUN: not FileCheck -color %s < %t.in 2> %t.color
+
+; Check whether color was produced.
+; RUN: FileCheck -check-prefix NO-COLOR %s < %t.no-color
+; RUN: FileCheck -check-prefix COLOR %s < %t.color
+
+; Make sure our NO-COLOR and COLOR patterns are sane: they don't match the
+; opposite cases.
+; RUN: not FileCheck -check-prefix COLOR %s < %t.no-color
+; RUN: not FileCheck -check-prefix NO-COLOR %s < %t.color
+
+; I don't know of a good way to check for ANSI color codes, so just make sure
+; some new characters show up where those codes should appear.
+; NO-COLOR: : error: CHECK: expected string not found in input
+; COLOR: : {{.+}}error: {{.+}}CHECK: expected string not found in input

llvm/trunk/utils/FileCheck/FileCheck.cpp

 // This program exits with an exit status of 2 on error, exit status of 0 if
 // the file matched the expected contents, and exit status of 1 if it did not
 // contain the expected contents.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/FileCheck.h"
 using namespace llvm;
 
 static cl::opt<std::string>
     CheckFilename(cl::Positional, cl::desc("<check-file>"), cl::Required);
 
 static cl::opt<std::string>

 static void DumpCommandLine(int argc, char **argv) {
   errs() << "FileCheck command line: ";
   for (int I = 0; I < argc; I++)
     errs() << " " << argv[I];
   errs() << "\n";
 }
 
 int main(int argc, char **argv) {
+  // Enable use of ANSI color codes because FileCheck is using them to
+  // highlight text.
+  llvm::sys::Process::UseANSIEscapeCodes(true);
+
   InitLLVM X(argc, argv);
   cl::ParseCommandLineOptions(argc, argv);
 
   FileCheckRequest Req;
   for (auto Prefix : CheckPrefixes)
     Req.CheckPrefixes.push_back(Prefix);
 
   for (auto CheckNot : ImplicitCheckNot)