[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev created this revision.
hyd-dev added reviewers: erik.pilkington, rsmith.
hyd-dev added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.
hyd-dev edited the summary of this revision.
hyd-dev edited the summary of this revision.

D62825  adds the new `BuiltinBitCastExpr`, but 
does not set the `Code` member of `ASTStmtWriter`. This is not correct and 
causes an assertion failue (`assert(Code != serialization::STMT_NULL_PTR && 
"unhandled sub-statement writing AST file")`) in `ASTStmtWriter::emit()` when 
building PCHs which contain `__builtin_bit_cast`. This patch adds 
`serialization::EXPR_BUILTIN_BIT_CAST` and handles `ASTStmtWriter::Code` 
properly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80360

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/PCH/builtin-bit-cast.cpp


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template
+constexpr T builtin_bit_cast_wrapper(const U& arg) {
+  return __builtin_bit_cast(T, arg);
+}
+
+#else
+
+int main() {
+  builtin_bit_cast_wrapper(0);
+  return 0;
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3593,6 +3593,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  S = new (Context) BuiltinBitCastExpr(
+  Empty, /*PathSize*/ Record[ASTStmtReader::NumExprFields]);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1791,6 +1791,9 @@
   /// A CXXFunctionalCastExpr record.
   EXPR_CXX_FUNCTIONAL_CAST,
 
+  /// A BuiltinBitCastExpr record.
+  EXPR_BUILTIN_BIT_CAST,
+
   /// A UserDefinedLiteral record.
   EXPR_USER_DEFINED_LITERAL,
 
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4785,6 +4785,8 @@
   : ExplicitCastExpr(BuiltinBitCastExprClass, T, VK, CK, SrcExpr, 0,
  DstType),
 KWLoc(KWLoc), RParenLoc(RParenLoc) {}
+  BuiltinBitCastExpr(EmptyShell Empty, unsigned PathSize)
+  : ExplicitCastExpr(BuiltinBitCastExprClass, Empty, PathSize) {}
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return KWLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template
+constexpr T builtin_bit_cast_wrapper(const U& arg) {
+  return __builtin_bit_cast(T, arg);
+}
+
+#else
+
+int main() {
+  builtin_bit_cast_wrapper(0);
+  return 0;
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.

[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 265436.
hyd-dev added a comment.

Format the test `builtin-bit-cast.cpp`.


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

https://reviews.llvm.org/D80360

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/PCH/builtin-bit-cast.cpp


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3593,6 +3593,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  S = new (Context) BuiltinBitCastExpr(
+  Empty, /*PathSize*/ Record[ASTStmtReader::NumExprFields]);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1791,6 +1791,9 @@
   /// A CXXFunctionalCastExpr record.
   EXPR_CXX_FUNCTIONAL_CAST,
 
+  /// A BuiltinBitCastExpr record.
+  EXPR_BUILTIN_BIT_CAST,
+
   /// A UserDefinedLiteral record.
   EXPR_USER_DEFINED_LITERAL,
 
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4785,6 +4785,8 @@
   : ExplicitCastExpr(BuiltinBitCastExprClass, T, VK, CK, SrcExpr, 0,
  DstType),
 KWLoc(KWLoc), RParenLoc(RParenLoc) {}
+  BuiltinBitCastExpr(EmptyShell Empty, unsigned PathSize)
+  : ExplicitCastExpr(BuiltinBitCastExprClass, Empty, PathSize) {}
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return KWLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3593,6 +3593,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  S = new (Context) BuiltinBitCastExpr(
+  Empty, /*PathSize*/ Record[ASTStmtReader::NumExprFields]);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

Fix typos in the summary.


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

**Ping?**


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

https://reviews.llvm.org/D80360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

**Ping?**


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

If this is OK, please commit it with `--author "hyd-dev 
"`.


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

https://reviews.llvm.org/D80360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 269765.
hyd-dev marked an inline comment as done.
hyd-dev added a comment.

Address the comment: remove `PathSize` from `BuiltinBitCastExpr`'s constructor 
and `assert()` it `== 0`.


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

https://reviews.llvm.org/D80360

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/PCH/builtin-bit-cast.cpp


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1646,6 +1646,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3606,6 +3606,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  assert(Record[ASTStmtReader::NumExprFields] == 0 && "Wrong PathSize!");
+  S = new (Context) BuiltinBitCastExpr(Empty);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1797,6 +1797,9 @@
   /// A CXXFunctionalCastExpr record.
   EXPR_CXX_FUNCTIONAL_CAST,
 
+  /// A BuiltinBitCastExpr record.
+  EXPR_BUILTIN_BIT_CAST,
+
   /// A UserDefinedLiteral record.
   EXPR_USER_DEFINED_LITERAL,
 
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4822,6 +4822,8 @@
   : ExplicitCastExpr(BuiltinBitCastExprClass, T, VK, CK, SrcExpr, 0,
  DstType),
 KWLoc(KWLoc), RParenLoc(RParenLoc) {}
+  BuiltinBitCastExpr(EmptyShell Empty)
+  : ExplicitCastExpr(BuiltinBitCastExprClass, Empty, 0) {}
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return KWLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1646,6 +1646,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3606,6 +3606,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  assert(Record[ASTStmtReader::NumExprFields] == 0 && "Wrong PathSize!");
+  S = new (Context) BuiltinBitCastExpr(Empty);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBi

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

**Ping?**


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

I don't have commit access. @njames93 Could you please commit it for me 
(`hyd-dev `)? Thanks!


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --color-diagnostics option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
hyd-dev edited projects, added clang-tools-extra; removed clang.
Herald added a project: clang.
hyd-dev removed a project: clang.
Herald added a project: clang.

This patch add `--color-diagnostics` to clang-tidy to control colors in 
diagnostics. With this option, users can force colorful output. This is useful 
when using clang-tidy with parallelization command line tools (like ninja and 
GNU parallel), as they often pipe clang-tidy's standard output, which make 
colors disappear.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79477

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,6 +76,7 @@
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  ColorDiagnostics: false
   )");
   ASSERT_TRUE(!!Options1);
   llvm::ErrorOr Options2 = parseConfiguration(R"(
@@ -85,6 +86,7 @@
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  ColorDiagnostics: true
   )");
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
@@ -98,6 +100,8 @@
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
+  ASSERT_TRUE(Options.ColorDiagnostics.hasValue());
+  EXPECT_TRUE(*Options.ColorDiagnostics);
 }
 
 class TestCheck : public ClangTidyCheck {
Index: clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
@@ -0,0 +1,27 @@
+// RUN: clang-tidy -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -dump-config -color-diagnostics | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -dump-config -color-diagnostics=false | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -config='ColorDiagnostics: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -config='ColorDiagnostics: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// REQUIRES: ansi-escape-sequences
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -color-diagnostics=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -color-diagnostics %s | FileCheck -check-prefix=CHECK-COLOR %s
+
+// CHECK-CONFIG-NO-COLOR: ColorDiagnostics: false
+// CHECK-CONFIG-COLOR: ColorDiagnostics: true
+// CHECK-OPT-PRESENT: --color-diagnostics
+
+class Base {
+public:
+  virtual ~Base() = 0;
+};
+
+class Delivered : public Base {
+public:
+  virtual ~Delivered() = default;
+  // CHECK-NO-COLOR: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-COLOR: {{.\[0;1;35m}}warning: {{.\[0m}}{{.\[1m}}prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+};
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -229,6 +229,13 @@
cl::value_desc("filename"),
cl::cat(ClangTidyCategory));
 
+static cl::opt ColorDiagnostics("color-diagnostics", cl::desc(R"(
+Use colors in diagnostics. If not set, colors
+will be used if standard output is connected
+to a terminal.
+)"),
+  cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -279,6 +286,7 @@
   // USERNAME is used on Windows.
   if (!DefaultOptions.User)
 DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
+  DefaultOptions.ColorDiagnostics = ColorDiagnostics;
 
   ClangTidyOptions OverrideOptions;
   if (Checks.getNumOccurrences() > 0)
@@ -291,6 +299,8 @@

[PATCH] D79477: [clang-tidy] Add --color-diagnostics command line option and ColorDiagnostics option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 262537.
hyd-dev added a comment.

Rename `--color-diagnostics` and `ColorDiagnostics` to `--use-color` and 
`UseColor`.  Also, improve the comment of `UseColor` option and the help text 
of `--use-color`, and add `cl::init(false)` to `cl::opt UseColor`.


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

https://reviews.llvm.org/D79477

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,6 +76,7 @@
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  UseColor: false
   )");
   ASSERT_TRUE(!!Options1);
   llvm::ErrorOr Options2 = parseConfiguration(R"(
@@ -85,6 +86,7 @@
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  UseColor: true
   )");
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
@@ -98,6 +100,8 @@
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
+  ASSERT_TRUE(Options.UseColor.hasValue());
+  EXPECT_TRUE(*Options.UseColor);
 }
 
 class TestCheck : public ClangTidyCheck {
Index: clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-tidy -dump-config | FileCheck %s
+// RUN: clang-tidy -dump-config -use-color | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -dump-config -use-color=false | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -config='UseColor: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// REQUIRES: ansi-escape-sequences
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+
+// CHECK-NOT: UseColor
+// CHECK-CONFIG-NO-COLOR: UseColor: false
+// CHECK-CONFIG-COLOR: UseColor: true
+// CHECK-OPT-PRESENT: --use-color
+
+class Base {
+public:
+  virtual ~Base() = 0;
+};
+
+class Delivered : public Base {
+public:
+  virtual ~Delivered() = default;
+  // CHECK-NO-COLOR: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-COLOR: {{.\[0;1;35m}}warning: {{.\[0m}}{{.\[1m}}prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+};
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -229,6 +229,13 @@
cl::value_desc("filename"),
cl::cat(ClangTidyCategory));
 
+static cl::opt UseColor("use-color", cl::desc(R"(
+Use colors in diagnostics. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.
+)"),
+  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -291,6 +298,8 @@
 OverrideOptions.SystemHeaders = SystemHeaders;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (UseColor.getNumOccurrences() > 0)
+OverrideOptions.UseColor = UseColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -126,6 +126,9 @@
   /// apply this config file on top of the parent one. If false or missing,
   /// only this configuration file will be used.
   llv

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 262541.
hyd-dev retitled this revision from "[clang-tidy] Add --color-diagnostics 
command line option and ColorDiagnostics option to control colors in 
diagnostics" to "[clang-tidy] Add --use-color command line option and UseColor 
option to control colors in diagnostics".
hyd-dev edited the summary of this revision.
hyd-dev added a comment.

Rename `clang-tools-extra/test/clang-tidy/infrastructure/color-diagnostics.cpp` 
to `clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp`.


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

https://reviews.llvm.org/D79477

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,6 +76,7 @@
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  UseColor: false
   )");
   ASSERT_TRUE(!!Options1);
   llvm::ErrorOr Options2 = parseConfiguration(R"(
@@ -85,6 +86,7 @@
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  UseColor: true
   )");
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
@@ -98,6 +100,8 @@
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
+  ASSERT_TRUE(Options.UseColor.hasValue());
+  EXPECT_TRUE(*Options.UseColor);
 }
 
 class TestCheck : public ClangTidyCheck {
Index: clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-tidy -dump-config | FileCheck %s
+// RUN: clang-tidy -dump-config -use-color | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -dump-config -use-color=false | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -config='UseColor: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// REQUIRES: ansi-escape-sequences
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+
+// CHECK-NOT: UseColor
+// CHECK-CONFIG-NO-COLOR: UseColor: false
+// CHECK-CONFIG-COLOR: UseColor: true
+// CHECK-OPT-PRESENT: --use-color
+
+class Base {
+public:
+  virtual ~Base() = 0;
+};
+
+class Delivered : public Base {
+public:
+  virtual ~Delivered() = default;
+  // CHECK-NO-COLOR: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-COLOR: {{.\[0;1;35m}}warning: {{.\[0m}}{{.\[1m}}prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+};
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -229,6 +229,13 @@
cl::value_desc("filename"),
cl::cat(ClangTidyCategory));
 
+static cl::opt UseColor("use-color", cl::desc(R"(
+Use colors in diagnostics. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.
+)"),
+  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -291,6 +298,8 @@
 OverrideOptions.SystemHeaders = SystemHeaders;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (UseColor.getNumOccurrences() > 0)
+OverrideOptions.UseColor = UseColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===
--- clang-tools-extra/clang-tidy/ClangT

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:99
 IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
+IO.mapOptional("UseColor", Options.UseColor);
   }

njames93 wrote:
> I'm not entirely sure this option belongs in the config file. However if it 
> will get read in here, maybe update the help string for the command line 
> option to say it overrides the `UseColor` option from the `.clang-tidy` 
> configuration file.
> I'm not entirely sure this option belongs in the config file.
It will be useful in the config file if the user invokes clang-tidy via another 
tool that does not allow the user to set clang-tidy's command line options.
> However if it will get read in here, maybe update the help string for the 
> command line option to say it overrides the `UseColor` option from the 
> `.clang-tidy` configuration file.
I've updated it.


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 262817.
hyd-dev marked an inline comment as done.
hyd-dev added a comment.

Say the `--use-color` command line option overrides the `UseColor` option in 
`.clang-tidy` file in its help text.


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

https://reviews.llvm.org/D79477

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,6 +76,7 @@
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  UseColor: false
   )");
   ASSERT_TRUE(!!Options1);
   llvm::ErrorOr Options2 = parseConfiguration(R"(
@@ -85,6 +86,7 @@
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  UseColor: true
   )");
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
@@ -98,6 +100,8 @@
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
+  ASSERT_TRUE(Options.UseColor.hasValue());
+  EXPECT_TRUE(*Options.UseColor);
 }
 
 class TestCheck : public ClangTidyCheck {
Index: clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-tidy -dump-config | FileCheck %s
+// RUN: clang-tidy -dump-config -use-color | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -dump-config -use-color=false | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -config='UseColor: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// REQUIRES: ansi-escape-sequences
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+
+// CHECK-NOT: UseColor
+// CHECK-CONFIG-NO-COLOR: UseColor: false
+// CHECK-CONFIG-COLOR: UseColor: true
+// CHECK-OPT-PRESENT: --use-color
+
+class Base {
+public:
+  virtual ~Base() = 0;
+};
+
+class Delivered : public Base {
+public:
+  virtual ~Delivered() = default;
+  // CHECK-NO-COLOR: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-COLOR: {{.\[0;1;35m}}warning: {{.\[0m}}{{.\[1m}}prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+};
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -229,6 +229,15 @@
cl::value_desc("filename"),
cl::cat(ClangTidyCategory));
 
+static cl::opt UseColor("use-color", cl::desc(R"(
+Use colors in diagnostics. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.
+This option overrides the 'UseColor' option in
+.clang-tidy file, if any.
+)"),
+  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -291,6 +300,8 @@
 OverrideOptions.SystemHeaders = SystemHeaders;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (UseColor.getNumOccurrences() > 0)
+OverrideOptions.UseColor = UseColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -126,6 +126,9 @@
   /// apply this config file on top of the parent one. If false or missing,
   /// only this configuration file will be used.
   llvm::Optional 

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 262887.
hyd-dev marked an inline comment as done.
hyd-dev added a comment.

Change incorrect `detect` to `detected` in the comment of `UseColor` option.


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

https://reviews.llvm.org/D79477

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -76,6 +76,7 @@
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  UseColor: false
   )");
   ASSERT_TRUE(!!Options1);
   llvm::ErrorOr Options2 = parseConfiguration(R"(
@@ -85,6 +86,7 @@
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  UseColor: true
   )");
   ASSERT_TRUE(!!Options2);
   ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
@@ -98,6 +100,8 @@
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
 llvm::join(Options.ExtraArgsBefore->begin(),
Options.ExtraArgsBefore->end(), ","));
+  ASSERT_TRUE(Options.UseColor.hasValue());
+  EXPECT_TRUE(*Options.UseColor);
 }
 
 class TestCheck : public ClangTidyCheck {
Index: clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-tidy -dump-config | FileCheck %s
+// RUN: clang-tidy -dump-config -use-color | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -dump-config -use-color=false | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -config='UseColor: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-COLOR %s
+// RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-COLOR %s
+// RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
+
+// REQUIRES: ansi-escape-sequences
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 -use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+
+// CHECK-NOT: UseColor
+// CHECK-CONFIG-NO-COLOR: UseColor: false
+// CHECK-CONFIG-COLOR: UseColor: true
+// CHECK-OPT-PRESENT: --use-color
+
+class Base {
+public:
+  virtual ~Base() = 0;
+};
+
+class Delivered : public Base {
+public:
+  virtual ~Delivered() = default;
+  // CHECK-NO-COLOR: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-COLOR: {{.\[0;1;35m}}warning: {{.\[0m}}{{.\[1m}}prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+};
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -229,6 +229,15 @@
cl::value_desc("filename"),
cl::cat(ClangTidyCategory));
 
+static cl::opt UseColor("use-color", cl::desc(R"(
+Use colors in diagnostics. If not set, colors
+will be used if the terminal connected to
+standard output supports colors.
+This option overrides the 'UseColor' option in
+.clang-tidy file, if any.
+)"),
+  cl::init(false), cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -291,6 +300,8 @@
 OverrideOptions.SystemHeaders = SystemHeaders;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (UseColor.getNumOccurrences() > 0)
+OverrideOptions.UseColor = UseColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -126,6 +126,9 @@
   /// apply this config file on top of the parent one. If false or missing,
   /// only this configuration file will be used.
   llvm::Optional InheritParentConfig;
+
+  /// Use colo

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

None of the clang-tidy command line options are prefixed with `-f`.
This command line option used to be `--color-diagnostics`, but I've followed 
@hokein's advice to change it to `--use-color`.
If you mean to use the `-fcolor-diagnostics` option from 
`compile_commands.json`, clang-tidy and clang are two separate tools. The users 
may not want them to share the same setting, because they may invoke them in 
different environments.


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

> Fair point, will this option also control the color of the diagnostics 
> emitted by clang or just clang tidy specific diagnostics?

This option sets `DiagOpts->ShowColors` to `true`. As I known, it controls 
**all** diagnostics reported by the `clang-tidy` program (by 
`clang::tidy::(anonymous namespace)::ErrorReporter`), including 
`clang-diagnostic-*` and other clang-tidy checks.


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

> Would a test case be needed?

`clang::tidy::clangTidyMain()` shows that clang-tidy reports all diagnostics by 
 `clang::tidy::handleErrors()`, which constructs a `clang::tidy::(anonymous 
namespace)::ErrorReporter`, no matter where they come from, so I think a test 
case is not required.

> Not a fan of this test case as it only demonstrates the color behaviour of 
> the process running the check not the actual option itself

What does "option itself" mean?


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

In D79477#2028637 , @njames93 wrote:

> What I mean to say is, if the behaviour of the testing environment changes to 
> pipe the result to a terminal that supports color, it could cause this test 
> case to also fail.


The standard output is always piped to `FileCheck`'s standard input. It will 
never become "a terminal that supports color".
Standard error is not redirected explicitly, but it is not used to detect color 
support.


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev marked 2 inline comments as done.
hyd-dev added a comment.

Any reply?


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

https://reviews.llvm.org/D79477



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits