[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Herald added a reviewer: njames93.

Crash no longer happens on main, but this change introduce nice fix for 
operators handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

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

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 558074.
PiotrZSL added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(&D::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return 
PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(&D::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(&D::operator(), &dd, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = &dd] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(&D::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(&D::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { return d->operator bool(); }
+
+auto MMM = std::bind(&E::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto MMM = [this] { return (*this)(1, 2); }
   }
 };
 
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -691,12 +691,15 @@
 const auto *MethodDecl = dyn_cast(LP.Callable.Decl);
 const BindArgument &ObjPtr = FunctionCallArgs.front();
 
-if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
-  Stream << ObjPtr.UsageIdentifier;
-  Stream << "->";
+if (MethodDecl->getOverloadedOperator() == OO_Call) {
+  Stream << "(*" << ObjPtr.UsageIdentifier << ')';
+} else {
+  if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
+Stream << ObjPtr.UsageIdentifier;
+Stream << "->";
+  }
+  Stream << MethodDecl->getNameAsString();
 }
-
-Stream << MethodDecl->getName();
   } else {
 switch (LP.Callable.CE) {
 case CE_Var:


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(&D::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(&D::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(&D::operator(), &dd, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = &dd] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(&D::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(&D::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { re

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

This change is dangerous, it depends that elements/iterators won't be 
re-allocated/invalidated during insertion process.
Part with removing double lookup in IncludeSorter::addInclude looks fine, but 
part that involve IncludeBucket is problematic.
I will try to push part that involve IncludeLocations optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117460

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

[PATCH] D130639: [clang-tidy] Fix readability-redundant-string-c-str fix for overloaded operator->

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D145730 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130639

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

[PATCH] D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D157326 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115106

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

[PATCH] D119165: [clang-tidy] Add processing lambda captures at bugprone-use-after-move check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed by D126780 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119165

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

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

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

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 558075.
PiotrZSL added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -77,7 +77,7 @@
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
   sum += sizeof(AsBool());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(AsInt());
@@ -103,41 +103,41 @@
   sum += sizeof(LEN + - + -sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(char) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
   sum += sizeof(A) / sizeof(S);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(B[0]) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(char*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(void*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(const void volatile*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(int) * sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  // CHECK-MESSAGES: :[[@LINE-1

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd867f668672d: [clang-tidy] modernize-avoid-bind: Fix 
handling of operators (authored by jspam, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125949

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(&D::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return 
PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(&D::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(&D::operator(), &dd, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = &dd] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(&D::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(&D::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto LLL = [d] { return d->operator bool(); }
+
+auto MMM = std::bind(&E::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto MMM = [this] { return (*this)(1, 2); }
   }
 };
 
Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
@@ -691,12 +691,15 @@
 const auto *MethodDecl = dyn_cast(LP.Callable.Decl);
 const BindArgument &ObjPtr = FunctionCallArgs.front();
 
-if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
-  Stream << ObjPtr.UsageIdentifier;
-  Stream << "->";
+if (MethodDecl->getOverloadedOperator() == OO_Call) {
+  Stream << "(*" << ObjPtr.UsageIdentifier << ')';
+} else {
+  if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
+Stream << ObjPtr.UsageIdentifier;
+Stream << "->";
+  }
+  Stream << MethodDecl->getNameAsString();
 }
-
-Stream << MethodDecl->getName();
   } else {
 switch (LP.Callable.CE) {
 case CE_Var:


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-bind.cpp
@@ -43,6 +43,7 @@
 struct D {
   D() = default;
   void operator()(int x, int y) const {}
+  operator bool() const { return true; }
 
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
@@ -342,6 +343,7 @@
 struct E {
   void MemberFunction(int x) {}
   int MemberFunctionWithReturn(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -379,6 +381,26 @@
 auto HHH = std::bind(&D::MemberFunctionWithReturn, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto HHH = [](auto && PH1) { return PH1->MemberFunctionWithReturn(1); };
+
+auto III = std::bind(&D::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [d] { (*d)(1, 2); }
+
+auto JJJ = std::bind(&D::operator(), &dd, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto JJJ = [ObjectPtr = &dd] { (*ObjectPtr)(1, 2); }
+
+auto KKK = std::bind(&D::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto KKK = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto LLL = std::bind(&D::operato

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e55fef0e98f: [clang-tidy] Tweak diag ranges for 
bugprone-sizeof-expression (authored by njames93, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101617

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -77,7 +77,7 @@
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
   sum += sizeof(AsBool());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(AsInt());
@@ -103,41 +103,41 @@
   sum += sizeof(LEN + - + -sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(char) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
   sum += sizeof(A) / sizeof(S);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(char) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(B[0]) / sizeof(A);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(char*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(void*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(const void volatile*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
   sum += sizeof(ptr) / sizeof(char);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(ptr) / sizeof(ptr[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
   sum += sizeof(int) * sizeof

[PATCH] D158657: [clang-tidy] Fix false-positives in misc-static-assert caused by non-constexpr variables

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158657

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

[PATCH] D144429: [clang-tidy] Add bugprone-chained-comparison check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144429

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

[PATCH] D33531: Clang-tidy readability: avoid const value return

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL marked 4 inline comments as done.
PiotrZSL added a comment.

Obsolete, this check is already added.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33531

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

[PATCH] D142565: [clang-tidy] Fix warning in portability-simd-intrinsics

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan created this revision.
Herald added subscribers: carlosgalvezp, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan updated this revision to Diff 492167.
ClockMan added a comment.
ClockMan updated this revision to Diff 492169.
Eugene.Zelenko added reviewers: aaron.ballman, LegalizeAdulthood, 
carlosgalvezp, gribozavr2.
ClockMan published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Corrected -std= in simd-intrinsics-ppc test


ClockMan added a comment.

Used same -check-suffix= in x86 and ppc tests


ClockMan added a comment.

Ready for review.


When portability-simd-intrinsics.Suggest were set to false,
produced warning were missing source location,
and due to that such warning coudn't be NOLINTed.

Added missing tests.

Fixes issues: https://github.com/llvm/llvm-project/issues/52831


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142565

Files:
  clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
  clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-simd-intrinsics 
%t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: portability-simd-intrinsics.Suggest, value: false} \
+// RUN:  ]}' -- -target x86_64
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics -check-suffix=BEFORE-CXX20 %t -- \
 // RUN:  -config='{CheckOptions: [ \
 // RUN:{key: portability-simd-intrinsics.Suggest, value: true} \
 // RUN:  ]}' -- -target x86_64
@@ -21,8 +25,9 @@
   __m256 d0;
 
   _mm_add_epi32(i0, i1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced 
by operator+ on std::experimental::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES-BEFORE-CXX20: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can 
be replaced by operator+ on std::experimental::simd objects 
[portability-simd-intrinsics]
   // CHECK-MESSAGES-CXX20: :[[@LINE-2]]:3: warning: '_mm_add_epi32' can be 
replaced by operator+ on std::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: '_mm_add_epi32' is a 
non-portable x86_64 intrinsic function [portability-simd-intrinsics]
   d0 = _mm256_load_pd(0);
   _mm256_store_pd(0, d0);
 
Index: 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-simd-intrinsics 
%t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: portability-simd-intrinsics.Suggest, value: false} \
+// RUN:  ]}' -- -target ppc64le -maltivec
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics -check-suffix=BEFORE-CXX20 %t -- \
 // RUN:  -config='{CheckOptions: [ \
 // RUN:{key: portability-simd-intrinsics.Suggest, value: true} \
 // RUN:  ]}' -- -target ppc64le -maltivec
@@ -13,6 +17,7 @@
   vector int i0, i1;
 
   vec_add(i0, i1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by 
operator+ on std::experimental::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES-BEFORE-CXX20: :[[@LINE-1]]:3: warning: 'vec_add' can be 
replaced by operator+ on std::experimental::simd objects 
[portability-simd-intrinsics]
   // CHECK-MESSAGES-CXX20: :[[@LINE-2]]:3: warning: 'vec_add' can be replaced 
by operator+ on std::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'vec_add' is a non-portable 
powerpc64le intrinsic function [portability-simd-intrinsics]
 }
Index: clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
===
--- clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
+++ clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
@@ -138,7 +138,7 @@
   << SimdRegex.sub(SmallString<32>({Std, "::simd"}),
StdRegex.sub(Std, New));
 } else {
-  diag("'%0' is a non-portable %1 intrinsic function")
+  diag(Call->getExprLoc(

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan published this revision for review.
ClockMan added a comment.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Ready for review.


Implemented support for bit-field members as a loop variable
or upper limit. Supporting also non bit-field integer members.

Fixes issues: https://github.com/llvm/llvm-project/issues/58614


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142587

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
   }
 }
 
+void voidBadForLoop7() {
+struct Int  {
+int value;
+} i;
+
+  for (i.value = 0; i.value < size(); ++i.value) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
 void voidForLoopUnsignedBound() {
   unsigned size = 3147483647;
   for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
   for (short i = 0; i < size; ++i) { // no warning
   }
 }
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 5;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+  }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 4;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 7;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+  struct StructWithBitField {
+  int var : 5;
+  unsigned limit : 5;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 7;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  StructWithBitField* ptr = &value;
+
+  for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:31
+  // Consider functions only in header files.
+  if (!Result.SourceManager->getFilename(SrcBegin).ends_with(".h"))
+return;

Probably better would be to check if this isn't main source 
Result.SourceManager->isInMainFile(SrcBegin)



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:35
+  auto Loc = FullSourceLoc(SrcBegin, *Result.SourceManager);
+  if (Loc.getBufferData().starts_with("LIBC_INLINE"))
+return;

Maybe put this LIBC_INLINE into some configuration, so check could be used by 
someone else also for other macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

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

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:20
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}

or maybe even better:
Finder->addMatcher(functionDecl(unless(isExpansionInMainFile()), 
isInline()).bind("func_decl"), this);
Instead of line 26 and 32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

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

[PATCH] D142592: [clang-tidy][libc] Add an inline function checker for the libc project.

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:20
+void InlineFunctionDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(decl(functionDecl()).bind("func_decl"), this);
+}

carlosgalvezp wrote:
> ClockMan wrote:
> > or maybe even better:
> > Finder->addMatcher(functionDecl(unless(isExpansionInMainFile()), 
> > isInline()).bind("func_decl"), this);
> > Instead of line 26 and 32.
> I'm not sure that works - if we pass a header directly to clang-tidy, it will 
> consider it as the "main file", right? That's why the established pattern is 
> based on `HeaderFileExtensions`, please check out other checks.
Yes you right, but isInline still can be checked here.
As for HeaderFileExtensions never used it from both developer and user point of 
view.
When running clang-tidy on headers is still better simply create file with 
single include.
Maybe if there would be AST_MATCHER for HeaderFileExtensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142592

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

[PATCH] D142655: [WIP][clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExtensions options

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:149
+  Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
+  Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
   Options.HeaderFilterRegex = "";

what about .c++ ? like file.c++ for implementation and h++ for header.
Its listed on 
https://en.wikibooks.org/wiki/C%2B%2B_Programming/Programming_Languages/C%2B%2B/Code/File_Organization

I know that probably no-one uses them, but who knows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142655

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

[PATCH] D142565: [clang-tidy] Fix warning in portability-simd-intrinsics

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d02dd241816: [clang-tidy] Fix warning in 
portability-simd-intrinsics (authored by ClockMan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142565

Files:
  clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
  clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-simd-intrinsics 
%t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: portability-simd-intrinsics.Suggest, value: false} \
+// RUN:  ]}' -- -target x86_64
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics -check-suffix=BEFORE-CXX20 %t -- \
 // RUN:  -config='{CheckOptions: [ \
 // RUN:{key: portability-simd-intrinsics.Suggest, value: true} \
 // RUN:  ]}' -- -target x86_64
@@ -21,8 +25,9 @@
   __m256 d0;
 
   _mm_add_epi32(i0, i1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced 
by operator+ on std::experimental::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES-BEFORE-CXX20: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can 
be replaced by operator+ on std::experimental::simd objects 
[portability-simd-intrinsics]
   // CHECK-MESSAGES-CXX20: :[[@LINE-2]]:3: warning: '_mm_add_epi32' can be 
replaced by operator+ on std::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: '_mm_add_epi32' is a 
non-portable x86_64 intrinsic function [portability-simd-intrinsics]
   d0 = _mm256_load_pd(0);
   _mm256_store_pd(0, d0);
 
Index: 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-ppc.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-simd-intrinsics 
%t -- \
+// RUN:  -config='{CheckOptions: [ \
+// RUN:{key: portability-simd-intrinsics.Suggest, value: false} \
+// RUN:  ]}' -- -target ppc64le -maltivec
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
portability-simd-intrinsics -check-suffix=BEFORE-CXX20 %t -- \
 // RUN:  -config='{CheckOptions: [ \
 // RUN:{key: portability-simd-intrinsics.Suggest, value: true} \
 // RUN:  ]}' -- -target ppc64le -maltivec
@@ -13,6 +17,7 @@
   vector int i0, i1;
 
   vec_add(i0, i1);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by 
operator+ on std::experimental::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES-BEFORE-CXX20: :[[@LINE-1]]:3: warning: 'vec_add' can be 
replaced by operator+ on std::experimental::simd objects 
[portability-simd-intrinsics]
   // CHECK-MESSAGES-CXX20: :[[@LINE-2]]:3: warning: 'vec_add' can be replaced 
by operator+ on std::simd objects [portability-simd-intrinsics]
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'vec_add' is a non-portable 
powerpc64le intrinsic function [portability-simd-intrinsics]
 }
Index: clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
===
--- clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
+++ clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
@@ -138,7 +138,7 @@
   << SimdRegex.sub(SmallString<32>({Std, "::simd"}),
StdRegex.sub(Std, New));
 } else {
-  diag("'%0' is a non-portable %1 intrinsic function")
+  diag(Call->getExprLoc(), "'%0' is a non-portable %1 intrinsic function")
   << Old << llvm::Triple::getArchTypeName(Arch);
 }
   }


Index: clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/portability/simd-intrinsics-x86.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s portability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy -std=c++11-or-later %s portability-simd-intrinsics %t -- \
+// RUN:  -config='{CheckOptions: 

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:34
 
+static StringRef TrimFirstChar(StringRef x) { return x.substr(1); }
+

Looks like usecase of this is more like rtrim, then probably rtrim should be 
used instead of substr.
And probably better would be to replace entire cl::desc. instead of adding new 
additional wrap.

more like: 
static auto desc(llvm::StringRef description) { return 
cl::desc(description.rtrim()); }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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

[PATCH] D142816: [clang-tidy] Add --list-unique-checks command

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan created this revision.
Herald added subscribers: carlosgalvezp, arphaman, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan updated this revision to Diff 493024.
ClockMan added a comment.
ClockMan published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

clang-format


ClockMan added a comment.

Read for review.


Currently in clang-tidy 48 checks are registred more
than once under diffrent names. This causes issues
for end user, who by mistake could enable same check
multiple times. This commit provides command that
can be used by user to troubleshot this issue.

Read more: https://github.com/llvm/llvm-project/issues/49960


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142816

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/ClangTidyModule.cpp
  clang-tools-extra/clang-tidy/ClangTidyModule.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/list-unique-checks.cpp
@@ -0,0 +1,4 @@
+// RUN: mkdir -p %T/clang-tidy/list-unique-checks/
+// RUN: echo '{Checks: "-*,google-readability-braces-around-statements,hicpp-braces-around-statements,readability-braces-around-statements"}' > %T/clang-tidy/.clang-tidy
+// RUN: cd %T/clang-tidy/list-unique-checks
+// RUN: clang-tidy -list-unique-checks | grep "^ *google-readability-braces-around-statements, hicpp-braces-around-statements, readability-braces-around-statements"
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -139,7 +139,7 @@
  When the value is empty, clang-tidy will
  attempt to find a file named .clang-tidy for
  each source file in its parent directories.
---config-file= - 
+--config-file= -
 Specify the path of .clang-tidy or custom config file:
   e.g. --config-file=/some/path/myTidyConfigFile
 This option internally works exactly the same way as
@@ -217,6 +217,10 @@
 --list-checks  -
  List all enabled checks and exit. Use with
  -checks=* to list all available checks.
+--list-unique-checks   -
+ List all enabled checks and exit. Check
+ aliases are listed in the same line. Use
+ with -checks=* to list all available checks.
 -load= -
  Load the dynamic object ``plugin``. This
  object should register new static analyzer
@@ -237,7 +241,7 @@
  format to stderr. When this option is passed,
  these per-TU profiles are instead stored as JSON.
 --system-headers   - Display the errors from system headers.
---use-color- 
+--use-color-
 Use colors in diagnostics. If not set, colors
 will be used if the terminal connected to
 standard output supports colors.
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
@@ -130,10 +130,10 @@
cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt FixNotes("fix-notes", cl::desc(R"(
-If a warning has no fix, but a single fix can 
-be found through an associated diagnostic note, 
-apply the fix. 
-Specifying this flag will implicitly enable the 
+If a warning has no fix, but a single fix can
+be found through an associated diagnostic note,
+apply the fix.
+Specifying this flag will implicitly enable the
 '--fix' flag.
 )"),
   cl::init(false), cl::cat(ClangTidyCategory));
@@ -152,8 +152,8 @@
 This option overrides the 'FormatStyle` option in
 .clang-tidy file, if any.
 )"),
-   cl::init("none"),
-   cl::cat(ClangTidyCategory));
+ 

[PATCH] D142816: [clang-tidy] Add --list-unique-checks command

[Piotr Zegar via Phabricator via cfe-commits]

ClockMan abandoned this revision.
ClockMan added a comment.

Ok, I agree that solution taken in D114317  
with explicit marking of checks via misc::PrimaryCheck or conversion of type to 
string on preprocesor level is better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142816

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

[PATCH] D159436: [clang-tidy] Add support for optional parameters in config.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

Except few nits in documentation looks fine. Probably one day in future we can 
add same for bools.
Make sure that documentation of impacted checks is correct, so it says that 
argument is positive integer, or none to force default value.




Comment at: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h:20-34
+///   * `LineThreshold` - flag functions exceeding this number of lines. This
+/// parameter is disabled by default.
 ///   * `StatementThreshold` - flag functions exceeding this number of
 /// statements. This may differ significantly from the number of lines for
 /// macro-heavy code. The default is `800`.
 ///   * `BranchThreshold` - flag functions exceeding this number of control
+/// statements. This parameter is disabled by default.

Those options documentation could be removed from this file. Only keep check 
description.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:116-118
+- Added support for optional parameters. Parameters that previously used -1 to 
disable
+  their effect can now be set to `none`, `null`, or left empty to get the same
+  behaviour.

I'm not sure if this release notes entry is needed. Maybe this change should be 
documented per changed check basic.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst:15
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This parameter is disabled
+   by default.

Leave previous but change -1 into none.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

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

[PATCH] D127036: [clang-tidy] Warn about arrays in `bugprone-undefined-memory-manipulation`

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

LGTM, I will rebase, push it and add release notes.


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

https://reviews.llvm.org/D127036

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

[PATCH] D127036: [clang-tidy] Warn about arrays in `bugprone-undefined-memory-manipulation`

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6780d53f41fb: [clang-tidy] Warn about arrays in 
bugprone-undefined-memory-manipulation (authored by fwolff, committed by 
PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D127036?vs=434283&id=557139#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127036

Files:
  clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
@@ -126,6 +126,12 @@
   ::memmove(&p, &vb, sizeof(int));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source 
object type 'types::VirtualBase'
 
+  types::Copy ca[10];
+  memset(ca, 0, sizeof(ca));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'types::Copy[10]'
+  memset(&ca, 0, sizeof(ca));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'types::Copy[10]'
+
 #define MEMSET memset(&vf, 0, sizeof(int));
   MEMSET
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'types::VirtualFunc'
@@ -159,6 +165,17 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'aliases::Copy2'
   memset(pc3, 0, sizeof(int));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'Copy3'
+  using Copy3Arr = Copy3[5];
+  Copy3Arr c3a;
+  memset(c3a, 0, sizeof(c3a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'Copy3Arr'
+  memset(&c3a, 0, sizeof(c3a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'Copy3Arr'
+
+  typedef Copy3 Copy3Arr2[5];
+  Copy3Arr2 c3a2;
+  memset(c3a2, 0, sizeof(c3a2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination 
object type 'Copy3Arr2'
 }
 
 void triviallyCopyable() {
Index: 
clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
@@ -22,9 +22,14 @@
 } // namespace
 
 void UndefinedMemoryManipulationCheck::registerMatchers(MatchFinder *Finder) {
-  const auto NotTriviallyCopyableObject =
-  hasType(ast_matchers::hasCanonicalType(
-  pointsTo(cxxRecordDecl(isNotTriviallyCopyable();
+  const auto HasNotTriviallyCopyableDecl =
+  hasDeclaration(cxxRecordDecl(isNotTriviallyCopyable()));
+  const auto ArrayOfNotTriviallyCopyable =
+  arrayType(hasElementType(HasNotTriviallyCopyableDecl));
+  const auto NotTriviallyCopyableObject = hasType(hasCanonicalType(
+  anyOf(pointsTo(qualType(anyOf(HasNotTriviallyCopyableDecl,
+ArrayOfNotTriviallyCopyable))),
+ArrayOfNotTriviallyCopyable)));
 
   // Check whether destination object is not TriviallyCopyable.
   // Applicable to all three memory manipulation functions.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/undefined-memory-manipulation.cpp
@@ -126,6 +126,12 @@
   ::memmove(&p, &vb, sizeof(int));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, source object type 'types::VirtualBase'
 
+  types::Copy ca[10];
+  memset(ca, 0, sizeof(ca));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type 'types::Copy[10]'
+  memset(&ca, 0, sizeof(ca));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type 'types::Copy[10]'
+
 #define MEMSET memset(&vf, 0, sizeof(int));
   MEMSET
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type 'types::VirtualFunc'
@@ -159,6 +165,17 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type 'aliases::Copy2'
   memset(pc3, 0, sizeof(int));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type 'Copy3'
+  using Copy3Arr = Copy3[5];
+  Copy3Arr c3a;
+  memset(c3a, 0, sizeof(c3a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: undefined behavior, destination object type '

[PATCH] D109210: [obsolete][clang-tidy] Attach fixit to warning, not note, in add_new_check.py example

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

No longer needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109210

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

[PATCH] D76477: [clang-tidy] Update path of main translation unit

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.

LGTM, probably LLVM_ATTRIBUTE_UNUSED  need to be added to example before 
committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76477

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

[PATCH] D76477: [clang-tidy][NFC] Update path of main translation unit

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG37937b8a040c: [clang-tidy][NFC] Update path of main 
translation unit (authored by j-carl, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76477

Files:
  clang-tools-extra/docs/clang-tidy/Contributing.rst


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -475,7 +475,7 @@
 
 And this to the main translation unit of the :program:`clang-tidy` binary (or
 the binary you link the ``clang-tidy`` library in)
-``clang-tidy/tool/ClangTidyMain.cpp``:
+``clang-tidy/ClangTidyForceLinker.h``:
 
 .. code-block:: c++
 


Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -475,7 +475,7 @@
 
 And this to the main translation unit of the :program:`clang-tidy` binary (or
 the binary you link the ``clang-tidy`` library in)
-``clang-tidy/tool/ClangTidyMain.cpp``:
+``clang-tidy/ClangTidyForceLinker.h``:
 
 .. code-block:: c++
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

LGTM, but I'm not sure if isCopyableOrMovable will always work correctly, for a 
user defined special members it looks ok, but for some implicit ones I worry it 
may not always work. Probably things like "(hasSimpleCopyAssigment()) || 
(hasUserDeclaredCopyAssigment() && check here if its not deleted)" would be 
needed.
Thing is that CXXRecordDecl got most info (in confused way), there are things 
like DefaultedMoveConstructorIsDeleted that could be used to verify somehow 
base class.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:91-102
+  Finder->addMatcher(
+  fieldDecl(unless(isMemberOfLambda()),
+hasDeclContext(cxxRecordDecl(isCopyableOrMovable())),
+hasType(hasCanonicalType(referenceType(
+  .bind("ref"),
+  this);
+  Finder->addMatcher(

Check first type, should be cheaper and consider mering those two. 



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst:6
 
-This check warns when structs or classes have const-qualified or reference
-(lvalue or rvalue) data members. Having such members is rarely useful, and
-makes the class only copy-constructible but not copy-assignable.
+This check warns when structs or classes that are copyable or movable have
+const-qualified or reference (lvalue or rvalue) data members. Having such




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155625

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

[PATCH] D153423: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff40c3488177: [clang-tidy] Allow explicit throwing in 
bugprone-exception-escape for special… (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D153423?vs=534059&id=541727#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153423

Files:
  clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
  clang/include/clang/Basic/ExceptionSpecificationType.h

Index: clang/include/clang/Basic/ExceptionSpecificationType.h
===
--- clang/include/clang/Basic/ExceptionSpecificationType.h
+++ clang/include/clang/Basic/ExceptionSpecificationType.h
@@ -50,6 +50,11 @@
   return ESpecType == EST_Unevaluated || ESpecType == EST_Uninstantiated;
 }
 
+inline bool isExplicitThrowExceptionSpec(ExceptionSpecificationType ESpecType) {
+  return ESpecType == EST_Dynamic || ESpecType == EST_MSAny ||
+ ESpecType == EST_NoexceptFalse;
+}
+
 /// Possible results from evaluation of a noexcept expression.
 enum CanThrowResult {
   CT_Cannot,
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -722,3 +722,27 @@
   test_basic_throw();
 }
 
+namespace PR55143 { namespace PR40583 {
+
+struct test_explicit_throw {
+test_explicit_throw() throw(int) { throw 42; }
+test_explicit_throw(const test_explicit_throw&) throw(int) { throw 42; }
+test_explicit_throw(test_explicit_throw&&) throw(int) { throw 42; }
+test_explicit_throw& operator=(const test_explicit_throw&) throw(int) { throw 42; }
+test_explicit_throw& operator=(test_explicit_throw&&) throw(int) { throw 42; }
+~test_explicit_throw() throw(int) { throw 42; }
+};
+
+struct test_implicit_throw {
+test_implicit_throw() { throw 42; }
+test_implicit_throw(const test_implicit_throw&) { throw 42; }
+test_implicit_throw(test_implicit_throw&&) { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'test_implicit_throw' which should not throw exceptions
+test_implicit_throw& operator=(const test_implicit_throw&) { throw 42; }
+test_implicit_throw& operator=(test_implicit_throw&&) { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+~test_implicit_throw() { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function '~test_implicit_throw' which should not throw exceptions
+};
+
+}}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -183,7 +183,6 @@
 
 struct Evil {
   ~Evil() noexcept(false) {
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~Evil' which should not throw exceptions
 throw 42;
   }
 };
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
@@ -22,6 +22,12 @@
 operations are also used to create move operations. A throwing ``main()``
 function also results in unexpected termination.
 
+Functions declared explicitly with ``noexcept(false)`` or ``throw(exception)``
+will be excluded from the analysis, as even though it is not recommended for
+functions like ``swap()``, ``main()``, move constructors, move assignment operators
+and destructors, it is a clear indication of the developer's intention and
+should be respected.
+
 WARNING! This check may be expensive on large source files.
 
 Options
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -274,13 +274,10 @@
   Global options of the same name should be used instead.
 
 - Improved :doc:`bugprone-exception-escape
-  ` to not emit warnings for
-  forward declarations of functions and additionally modified it to skip
-  ``no

[PATCH] D134588: [clang-tidy] Fix bugprone-exception-escape warn on noexcept calls

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Already fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134588

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:91-102
+  Finder->addMatcher(
+  fieldDecl(unless(isMemberOfLambda()),
+hasDeclContext(cxxRecordDecl(isCopyableOrMovable())),
+hasType(hasCanonicalType(referenceType(
+  .bind("ref"),
+  this);
+  Finder->addMatcher(

carlosgalvezp wrote:
> PiotrZSL wrote:
> > Check first type, should be cheaper and consider mering those two. 
> Thanks for the tip! I'm not familiar with having multiple binds in the same 
> `addMatcher` call. Do I still need to keep the `bind("ref")` at the end?
No, bind at the end need to be removed (I forgot about that).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155625

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.

Looks fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155625

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

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

To be honest, all those things just should be configurable instead of 
hardcoding them.
For example, a project that I work for got 4 different custom optional 
implementations, and this check is useless for all of them.
And still no boost::optional support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890

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

[PATCH] D155321: [clang][Analysis] ExprMutationAnalyzer: break infinite recursion on recursive function call

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:31
+if (nullptr == DeclAnalyzed) {
+  OwningDeclAnalyzed = std::make_unique();
+  this->DeclAnalyzed = OwningDeclAnalyzed.get();

NOTE: This memory allocation could be removed by creating class 
ExprMutationAnalyzerWithHistory that could keep DeclSet as private object and 
use ExprMutationAnalyzer as base class.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:574
+}
+DeclAnalyzed->insert(Func);
+

I would assume that Func should be removed from DeclAnalyzed when leaving 
scope. Otherwise it may not work to good when for example same function would 
be called twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155321

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

[PATCH] D156024: [clang-tidy] Add --disable-modular-headers-expansion option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Experimental option: Disable modular header expansion
in C++20 and above. This option serves as a workaround
for addressing known issues #50087 and #62447.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156024

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


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
@@ -263,6 +263,16 @@
cl::init(false), cl::Hidden,
cl::cat(ClangTidyCategory));
 
+static cl::opt
+DisableModularHeadersExpansion("disable-modular-headers-expansion", 
desc(R"(
+Experimental option: Disable modular header
+expansion in C++20 and above. This option
+serves as a workaround for addressing known
+issues #50087 and #62447.
+)"),
+cl::init(false), cl::Hidden,
+cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -659,7 +669,8 @@
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
-   AllowEnablingAnalyzerAlphaCheckers);
+   AllowEnablingAnalyzerAlphaCheckers,
+   DisableModularHeadersExpansion);
   std::vector Errors =
   runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
FixNotes, EnableCheckProfile, ProfilePrefix);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -70,7 +70,8 @@
 public:
   /// Initializes \c ClangTidyContext instance.
   ClangTidyContext(std::unique_ptr OptionsProvider,
-   bool AllowEnablingAnalyzerAlphaCheckers = false);
+   bool AllowEnablingAnalyzerAlphaCheckers = false,
+   bool DisableModularHeadersExpansion = false);
   /// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
   // FIXME: this is required initialization, and should be a constructor param.
   // Fix the context -> diag engine -> consumer -> context initialization 
cycle.
@@ -198,6 +199,10 @@
 return AllowEnablingAnalyzerAlphaCheckers;
   }
 
+  bool canEnableModularHeadersExpansion() const {
+return !DisableModularHeadersExpansion;
+  }
+
   void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
 
   bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -245,6 +250,7 @@
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool DisableModularHeadersExpansion;
 
   bool SelfContainedDiags;
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -161,10 +161,12 @@
 
 ClangTidyContext::ClangTidyContext(
 std::unique_ptr OptionsProvider,
-bool AllowEnablingAnalyzerAlphaCheckers)
+bool AllowEnablingAnalyzerAlphaCheckers,
+bool DisableModularHeadersExpansion)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
   Profile(false),
   AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+  DisableModularHeadersExpansion(DisableModularHeadersExpansion),
   SelfContainedDiags(false) {
   // Before the first translation unit we can get errors related to 
command-line
   // parsing, use empty string for the file name in this case.
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -418,7 +418,7 @@
   Preprocessor *PP = &Compiler.getPreprocessor();
   Preprocessor *ModuleExpanderPP = PP;
 
-  if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+  if (Context.canEnableModularHeadersExpansion() && 
Context.getLangOpts().Modules && OverlayFS != nullptr) {
 auto ModuleExpander = std::make_unique(
 &Compiler, OverlayFS);

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543185.
PiotrZSL edited the summary of this revision.
PiotrZSL added a comment.

Add test & release notes.
Cleanup commit description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146520

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp


Index: 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
===
--- 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
+++ 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
@@ -1,12 +1,15 @@
-// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN: clang-tidy %s --checks='-*,llvm-namespace-comment,clang-diagnostic*' \
 // RUN:   -- -Wunused-variable 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WARN 
-implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s 
-checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s 
--checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN:   --warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR 
-implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s 
-checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 
2>&1 \
+// RUN: not clang-tidy %s 
--checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN:   --warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 
2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR-QUIET 
-implicit-check-not='{{warning|error}}:'
+// RUN: clang-tidy %s --checks='-*,llvm-namespace-comment' 
--warnings-as-errors=* -- \
+// RUN:   -Wunused-variable 2>&1 | FileCheck %s 
--check-prefix=CHECK-SUPPRESSED \
+// RUN:   -implicit-check-not='{{warning|error}}:'
 
 void f() { int i; }
 // CHECK-WARN: warning: unused variable 'i' [clang-diagnostic-unused-variable]
@@ -16,3 +19,7 @@
 // CHECK-WARN-NOT: treated as
 // CHECK-WERR: 1 warning treated as error
 // CHECK-WERR-QUIET-NOT: treated as
+
+// CHECK-SUPPRESSED-NOT: unused variable 'i'
+// CHECK-SUPPRESSED: 1 warning generated.
+// CHECK-SUPPRESSED: Suppressed 1 warnings (1 with check filters).
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,11 @@
 - Support specifying `SystemHeaders` in the `.clang-tidy` configuration file,
   with the same functionality as the command-line option `--system-headers`.
 
+- `WarningsAsErrors` (`--warnings-as-errors=`) no longer promotes unlisted
+  warnings to errors, using `Checks` (`--checks=`) as the filter. Use
+  `-Werror=` on the compiler command-line for custom error promotion
+  irrespective of `Checks` (`--checks=`) settings.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -421,8 +421,6 @@
 
 bool IsWarningAsError = DiagLevel == DiagnosticsEngine::Warning &&
 Context.treatAsError(CheckName);
-if (IsWarningAsError)
-  Level = ClangTidyError::Error;
 Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),
 IsWarningAsError);
   }
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -620,6 +620,8 @@
   TUD.MainSourceFile = std::string(MainFilePath);
   for (const auto &Error : Errors) {
 tooling::Diagnostic Diag = Error;
+if (Error.IsWarningAsError)
+  Diag.DiagLevel = tooling::Diagnostic::Error;
 TUD.Diagnostics.insert(TUD.Diagnostics.end(), Diag);
   }
 


Index: clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
@@ -1,12 +1,15 @@
-// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN: clang-tidy %s --checks='-*,llvm-namespace-comment,clang-dia

[PATCH] D144135: [clang-tidy] Add performance-enum-size check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

@Eugene.Zelenko I cannot review/accept my own patch :(.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135

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

[PATCH] D144135: [clang-tidy] Add performance-enum-size check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543196.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Rebase + Update diagnostic message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
  clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s performance-enum-size %t -- \
+// RUN:   -config="{CheckOptions: [{key: performance-enum-size.EnumIgnoreList, value: '::IgnoredEnum;IgnoredSecondEnum'}]}"
+
+namespace std
+{
+using uint8_t = unsigned char;
+using int8_t = signed char;
+using uint16_t = unsigned short;
+using int16_t = signed short;
+using uint32_t = unsigned int;
+using int32_t = signed int;
+}
+
+enum class Value
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'Value' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported
+};
+
+
+enum class EnumClass : std::int16_t
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'EnumClass' uses a larger base type ('std::int16_t' (aka 'short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported
+};
+
+enum EnumWithType : std::uint16_t
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithType' uses a larger base type ('std::uint16_t' (aka 'unsigned short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported,
+supported2
+};
+
+enum EnumWithNegative
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+s1 = -128,
+s2 = -100,
+s3 = 100,
+s4 = 127
+};
+
+enum EnumThatCanBeReducedTo2Bytes
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumThatCanBeReducedTo2Bytes' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+a1 = -128,
+a2 = 0x6EEE
+};
+
+enum EnumOnlyNegative
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumOnlyNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+b1 = -125,
+b2 = -50,
+b3 = -10
+};
+
+enum CorrectU8 : std::uint8_t
+{
+c01 = 10,
+c02 = 11
+};
+
+enum CorrectU16 : std::uint16_t
+{
+c11 = 10,
+c12 = 0x
+};
+
+constexpr int getValue()
+{
+return 256;
+}
+
+
+enum CalculatedDueToUnknown1 : unsigned int
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown1' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+c21 = 10,
+c22 = getValue()
+};
+
+enum CalculatedDueToUnknown2 : unsigned int
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown2' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+c31 = 10,
+c32 = c31 + 246
+};
+
+enum class IgnoredEnum : std::uint32_t
+{
+unused1 = 1,
+unused2 = 2
+};
+
+namespace internal
+{
+
+enum class IgnoredSecondEnum
+{
+unused1 = 1,
+unused2 = 2
+};
+
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - performance-enum-size
+
+performance-enum-size
+=
+
+Recommends the smallest possible underlying type for an ``enum`` or ``enum``
+class based on the range of its enumerators. Analyzes the value

[PATCH] D156024: [clang-tidy] Add --disable-modular-headers-expansion option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543199.
PiotrZSL added a comment.

Code formating


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156024

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


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
@@ -263,6 +263,16 @@
cl::init(false), cl::Hidden,
cl::cat(ClangTidyCategory));
 
+static cl::opt
+DisableModularHeadersExpansion("disable-modular-headers-expansion", 
desc(R"(
+Experimental option: Disable modular header
+expansion in C++20 and above. This option
+serves as a workaround for addressing known
+issues #50087 and #62447.
+)"),
+   cl::init(false), cl::Hidden,
+   cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -659,7 +669,8 @@
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
-   AllowEnablingAnalyzerAlphaCheckers);
+   AllowEnablingAnalyzerAlphaCheckers,
+   DisableModularHeadersExpansion);
   std::vector Errors =
   runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
FixNotes, EnableCheckProfile, ProfilePrefix);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -70,7 +70,8 @@
 public:
   /// Initializes \c ClangTidyContext instance.
   ClangTidyContext(std::unique_ptr OptionsProvider,
-   bool AllowEnablingAnalyzerAlphaCheckers = false);
+   bool AllowEnablingAnalyzerAlphaCheckers = false,
+   bool DisableModularHeadersExpansion = false);
   /// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
   // FIXME: this is required initialization, and should be a constructor param.
   // Fix the context -> diag engine -> consumer -> context initialization 
cycle.
@@ -198,6 +199,10 @@
 return AllowEnablingAnalyzerAlphaCheckers;
   }
 
+  bool canEnableModularHeadersExpansion() const {
+return !DisableModularHeadersExpansion;
+  }
+
   void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
 
   bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -245,6 +250,7 @@
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool DisableModularHeadersExpansion;
 
   bool SelfContainedDiags;
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -161,10 +161,12 @@
 
 ClangTidyContext::ClangTidyContext(
 std::unique_ptr OptionsProvider,
-bool AllowEnablingAnalyzerAlphaCheckers)
+bool AllowEnablingAnalyzerAlphaCheckers,
+bool DisableModularHeadersExpansion)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
   Profile(false),
   AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+  DisableModularHeadersExpansion(DisableModularHeadersExpansion),
   SelfContainedDiags(false) {
   // Before the first translation unit we can get errors related to 
command-line
   // parsing, use empty string for the file name in this case.
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -418,7 +418,8 @@
   Preprocessor *PP = &Compiler.getPreprocessor();
   Preprocessor *ModuleExpanderPP = PP;
 
-  if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+  if (Context.canEnableModularHeadersExpansion() &&
+  Context.getLangOpts().Modules && OverlayFS != nullptr) {
 auto ModuleExpander = std::make_unique(
 &Compiler, OverlayFS);
 ModuleExpanderPP = ModuleExpander->getPreprocessor();


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/too

[PATCH] D156031: [clang-tidy] Ignore implcit casts in cppcoreguidelines-owning-memory

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Add getCheckTraversalKind to the check in order
to ignore some implicit casts when matching
expresions.

Fixes: #63994


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156031

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
@@ -382,3 +382,16 @@
   template_function(IntOwner1);  // Ok, but not actually ok, since type 
deduction removes owner
   template_function(stack_ptr1); // Bad, but type deduction gets it wrong
 }
+
+namespace PR63994 {
+  struct A {
+virtual ~A() {}
+  };
+
+  struct B : public A {};
+
+  A* foo(int x) {
+return new B;
+// CHECK-NOTES: [[@LINE-1]]:5: warning: returning a newly created resource 
of type 'A *' or 'gsl::owner<>' from a function whose return type is not 
'gsl::owner<>'
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -330,6 +330,10 @@
   to emit warnings only on classes that are copyable/movable, as required by 
the
   corresponding rule.
 
+- Improved :doc:`cppcoreguidelines-owning-memory
+  ` check now finds more
+  issues, especially those related to implicit casts.
+
 - Deprecated C.48 enforcement from 
:doc:`cppcoreguidelines-prefer-member-initializer
   `. Please use
   :doc:`cppcoreguidelines-use-default-member-init
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
@@ -37,6 +37,9 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool handleDeletion(const ast_matchers::BoundNodes &Nodes);


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
@@ -382,3 +382,16 @@
   template_function(IntOwner1);  // Ok, but not actually ok, since type deduction removes owner
   template_function(stack_ptr1); // Bad, but type deduction gets it wrong
 }
+
+namespace PR63994 {
+  struct A {
+virtual ~A() {}
+  };
+
+  struct B : public A {};
+
+  A* foo(int x) {
+return new B;
+// CHECK-NOTES: [[@LINE-1]]:5: warning: returning a newly created resource of type 'A *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -330,6 +330,10 @@
   to emit warnings only on classes that are copyable/movable, as required by the
   corresponding rule.
 
+- Improved :doc:`cppcoreguidelines-owning-memory
+  ` check now finds more
+  issues, especially those related to implicit casts.
+
 - Deprecated C.48 enforcement from :doc:`cppcoreguidelines-prefer-member-initializer
   `. Please use
   :doc:`cppcoreguidelines-use-default-member-init
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
@@ -37,6 +37,9 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool handleDeletion(const ast_matchers::BoundNodes &Nodes);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailm

[PATCH] D135476: [clang-tidy] Support concepts in `bugprone-forwarding-reference-overload`

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543206.
PiotrZSL edited the summary of this revision.
PiotrZSL added a comment.

rebase, add release notes, move tests to separate file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135476

Files:
  clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s bugprone-forwarding-reference-overload %t
+
+template  constexpr bool just_true = true;
+
+class Test {
+public:
+  template  Test(T &&n);
+  // CHECK-NOTES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+
+  Test(const Test &rhs);
+  // CHECK-NOTES: :[[@LINE-1]]:3: note: copy constructor declared here
+};
+
+class Test1 {
+public:
+  // Guarded with requires expression.
+  template 
+  requires requires { just_true; }
+  Test1(T &&n);
+};
+
+template
+concept JustTrueConcept = requires { just_true; };
+
+class Test2 {
+public:
+  // Guarded with concept requirement.
+  template  Test2(T &&n);
+};
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
@@ -34,6 +34,15 @@
 enable_if_t, A&&...>, int> = 0>
   explicit Person(A&&... a) {}
 
+  // C5: perfect forwarding ctor guarded with requires expression
+  template
+  requires requires { is_special; }
+  explicit Person(T&& n) {}
+
+  // C6: perfect forwarding ctor guarded with concept requirement
+  template
+  explicit Person(T&& n) {}
+
   // (possibly compiler generated) copy ctor
   Person(const Person& rhs);
 };
@@ -42,8 +51,8 @@
 constructors. We suppress warnings if the copy and the move constructors are both
 disabled (deleted or private), because there is nothing the perfect forwarding
 constructor could hide in this case. We also suppress warnings for constructors
-like C3 and C4 that are guarded with an ``enable_if``, assuming the programmer was
-aware of the possible hiding.
+like C3-C6 that are guarded with an ``enable_if`` or a concept, assuming the
+programmer was aware of the possible hiding.
 
 Background
 --
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -283,6 +283,10 @@
   ` to handle iterators that do not
   define `value_type` type aliases.
 
+- Improved :doc:`bugprone-forwarding-reference-overload
+  ` check to ignore
+  constructors with associated constraints (C++ concepts).
+
 - Improved :doc:`bugprone-incorrect-roundings
   ` check by adding support for
   other floating point representations in float constant like ``0.5L``.
Index: clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
@@ -56,6 +56,9 @@
   return Node.hasDefaultArgument() &&
  TypeMatcher.matches(Node.getDefaultArgument(), Finder, Builder);
 }
+AST_MATCHER(TemplateDecl, hasAssociatedConstraints) {
+  return Node.hasAssociatedConstraints();
+}
 } // namespace
 
 void ForwardingReferenceOverloadCheck::registerMatchers(MatchFinder *Finder) {
@@ -74,6 +77,9 @@
   // No warning: enable_if as constructor parameter.
   parmVarDecl(hasType(isEnableIf(),
   unless(hasParent(functionTemplateDecl(anyOf(
+  // No warning: has associated constraints (like requires
+  // expression).
+  hasAssociatedConstraints(),
   // No warning: enable_if as type parameter.
   has(templateTypeParmDecl(hasDefaultArgument(isEnableIf(,
   // No warning: enable_if as non-type template parameter.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156031: [clang-tidy] Ignore implcit casts in cppcoreguidelines-owning-memory

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b5b75bfd4d4: [clang-tidy] Ignore implcit casts in 
cppcoreguidelines-owning-memory (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156031

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
@@ -382,3 +382,16 @@
   template_function(IntOwner1);  // Ok, but not actually ok, since type 
deduction removes owner
   template_function(stack_ptr1); // Bad, but type deduction gets it wrong
 }
+
+namespace PR63994 {
+  struct A {
+virtual ~A() {}
+  };
+
+  struct B : public A {};
+
+  A* foo(int x) {
+return new B;
+// CHECK-NOTES: [[@LINE-1]]:5: warning: returning a newly created resource 
of type 'A *' or 'gsl::owner<>' from a function whose return type is not 
'gsl::owner<>'
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -330,6 +330,10 @@
   to emit warnings only on classes that are copyable/movable, as required by 
the
   corresponding rule.
 
+- Improved :doc:`cppcoreguidelines-owning-memory
+  ` check now finds more
+  issues, especially those related to implicit casts.
+
 - Deprecated C.48 enforcement from 
:doc:`cppcoreguidelines-prefer-member-initializer
   `. Please use
   :doc:`cppcoreguidelines-use-default-member-init
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
@@ -37,6 +37,9 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool handleDeletion(const ast_matchers::BoundNodes &Nodes);


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp
@@ -382,3 +382,16 @@
   template_function(IntOwner1);  // Ok, but not actually ok, since type deduction removes owner
   template_function(stack_ptr1); // Bad, but type deduction gets it wrong
 }
+
+namespace PR63994 {
+  struct A {
+virtual ~A() {}
+  };
+
+  struct B : public A {};
+
+  A* foo(int x) {
+return new B;
+// CHECK-NOTES: [[@LINE-1]]:5: warning: returning a newly created resource of type 'A *' or 'gsl::owner<>' from a function whose return type is not 'gsl::owner<>'
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -330,6 +330,10 @@
   to emit warnings only on classes that are copyable/movable, as required by the
   corresponding rule.
 
+- Improved :doc:`cppcoreguidelines-owning-memory
+  ` check now finds more
+  issues, especially those related to implicit casts.
+
 - Deprecated C.48 enforcement from :doc:`cppcoreguidelines-prefer-member-initializer
   `. Please use
   :doc:`cppcoreguidelines-use-default-member-init
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.h
@@ -37,6 +37,9 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   bool handleDeletion(const ast_matchers::BoundNodes &Nodes);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146520: [clang-tidy] Fix checks filter with warnings-as-errors

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9644368f974a: [clang-tidy] Fix checks filter with 
warnings-as-errors (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D146520?vs=543185&id=543222#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146520

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp


Index: 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
===
--- 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
+++ 
clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
@@ -1,12 +1,15 @@
-// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN: clang-tidy %s --checks='-*,llvm-namespace-comment,clang-diagnostic*' \
 // RUN:   -- -Wunused-variable 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WARN 
-implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s 
-checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s 
--checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN:   --warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR 
-implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s 
-checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 
2>&1 \
+// RUN: not clang-tidy %s 
--checks='-*,llvm-namespace-comment,clang-diagnostic*' \
+// RUN:   --warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 
2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR-QUIET 
-implicit-check-not='{{warning|error}}:'
+// RUN: clang-tidy %s --checks='-*,llvm-namespace-comment' 
--warnings-as-errors=* -- \
+// RUN:   -Wunused-variable 2>&1 | FileCheck %s 
--check-prefix=CHECK-SUPPRESSED \
+// RUN:   -implicit-check-not='{{warning|error}}:'
 
 void f() { int i; }
 // CHECK-WARN: warning: unused variable 'i' [clang-diagnostic-unused-variable]
@@ -16,3 +19,7 @@
 // CHECK-WARN-NOT: treated as
 // CHECK-WERR: 1 warning treated as error
 // CHECK-WERR-QUIET-NOT: treated as
+
+// CHECK-SUPPRESSED-NOT: unused variable 'i'
+// CHECK-SUPPRESSED: 1 warning generated.
+// CHECK-SUPPRESSED: Suppressed 1 warnings (1 with check filters).
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -106,6 +106,11 @@
 - Support specifying `SystemHeaders` in the `.clang-tidy` configuration file,
   with the same functionality as the command-line option `--system-headers`.
 
+- `WarningsAsErrors` (`--warnings-as-errors=`) no longer promotes unlisted
+  warnings to errors. Only the warnings listed in `Checks` (`--checks=`) will
+  be promoted to errors. For custom error promotion, use `-Werror=`
+  on the compiler command-line, irrespective of `Checks` (`--checks=`) 
settings.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -421,8 +421,6 @@
 
 bool IsWarningAsError = DiagLevel == DiagnosticsEngine::Warning &&
 Context.treatAsError(CheckName);
-if (IsWarningAsError)
-  Level = ClangTidyError::Error;
 Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),
 IsWarningAsError);
   }
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -620,6 +620,8 @@
   TUD.MainSourceFile = std::string(MainFilePath);
   for (const auto &Error : Errors) {
 tooling::Diagnostic Diag = Error;
+if (Error.IsWarningAsError)
+  Diag.DiagLevel = tooling::Diagnostic::Error;
 TUD.Diagnostics.insert(TUD.Diagnostics.end(), Diag);
   }
 


Index: clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/warnings-as-errors-diagnostics.cpp
@@ -1,12 +1,15 @@
-// RUN: cla

[PATCH] D135476: [clang-tidy] Support concepts in `bugprone-forwarding-reference-overload`

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e325081192b: [clang-tidy] Support concepts in 
`bugprone-forwarding-reference-overload` (authored by Izaron, committed by 
PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135476

Files:
  clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/forwarding-reference-overload-concepts.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s bugprone-forwarding-reference-overload %t
+
+template  constexpr bool just_true = true;
+
+class Test {
+public:
+  template  Test(T &&n);
+  // CHECK-NOTES: :[[@LINE-1]]:25: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+
+  Test(const Test &rhs);
+  // CHECK-NOTES: :[[@LINE-1]]:3: note: copy constructor declared here
+};
+
+class Test1 {
+public:
+  // Guarded with requires expression.
+  template 
+  requires requires { just_true; }
+  Test1(T &&n);
+};
+
+template
+concept JustTrueConcept = requires { just_true; };
+
+class Test2 {
+public:
+  // Guarded with concept requirement.
+  template  Test2(T &&n);
+};
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/forwarding-reference-overload.rst
@@ -34,6 +34,15 @@
 enable_if_t, A&&...>, int> = 0>
   explicit Person(A&&... a) {}
 
+  // C5: perfect forwarding ctor guarded with requires expression
+  template
+  requires requires { is_special; }
+  explicit Person(T&& n) {}
+
+  // C6: perfect forwarding ctor guarded with concept requirement
+  template
+  explicit Person(T&& n) {}
+
   // (possibly compiler generated) copy ctor
   Person(const Person& rhs);
 };
@@ -42,8 +51,8 @@
 constructors. We suppress warnings if the copy and the move constructors are both
 disabled (deleted or private), because there is nothing the perfect forwarding
 constructor could hide in this case. We also suppress warnings for constructors
-like C3 and C4 that are guarded with an ``enable_if``, assuming the programmer was
-aware of the possible hiding.
+like C3-C6 that are guarded with an ``enable_if`` or a concept, assuming the
+programmer was aware of the possible hiding.
 
 Background
 --
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -288,6 +288,10 @@
   ` to handle iterators that do not
   define `value_type` type aliases.
 
+- Improved :doc:`bugprone-forwarding-reference-overload
+  ` check to ignore
+  constructors with associated constraints (C++ concepts).
+
 - Improved :doc:`bugprone-incorrect-roundings
   ` check by adding support for
   other floating point representations in float constant like ``0.5L``.
Index: clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
@@ -56,6 +56,9 @@
   return Node.hasDefaultArgument() &&
  TypeMatcher.matches(Node.getDefaultArgument(), Finder, Builder);
 }
+AST_MATCHER(TemplateDecl, hasAssociatedConstraints) {
+  return Node.hasAssociatedConstraints();
+}
 } // namespace
 
 void ForwardingReferenceOverloadCheck::registerMatchers(MatchFinder *Finder) {
@@ -74,6 +77,9 @@
   // No warning: enable_if as constructor parameter.
   parmVarDecl(hasType(isEnableIf(),
   unless(hasParent(functionTemplateDecl(anyOf(
+  // No warning: has associated constraints (like requires
+  // expression).
+  hasAssociatedConstraints(),
   // No warning: enable_if as type parameter.
   has(templateTypeParmDecl(hasDefaultArgument(isEnableIf(,
   // No warning: enable_if as non-type template parameter.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked 3 inline comments as done.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp:71
+
+bool EmptyCatchCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {

xgupta wrote:
> This can be defined in the header file itself like other checks.
Can be, but as this is already virtual function we do not gain anything by 
putting it into header. vtable will be emited anyway only on this TU.



Comment at: clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp:76
+
+std::optional EmptyCatchCheck::getCheckTraversalKind() const {
+  return TK_IgnoreUnlessSpelledInSource;

xgupta wrote:
> This can be defined in the header file itself like other checks.
Can be, but as this is already virtual function we do not gain anything by 
putting it into header. vtable will be emited anyway only on this TU.
And I want to keep it close to registerMatchers, as it impact behavior of that 
method.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp:3
+// RUN: -config="{CheckOptions: [{key: 
bugprone-empty-catch.AllowEmptyCatchForExceptions, value: 
'::SafeException;WarnException'}, \
+// RUN:{key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: 
'@IGNORE;@TODO'}]}"
+

xgupta wrote:
> If TODO is in default then it does not require to be in value here, right?
> I tested without that, it gives the warning. 
Setting `IgnoreCatchWithKeywords` manually override default value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

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

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543276.
PiotrZSL marked 3 inline comments as done.
PiotrZSL added a comment.

Rebase (release notes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:{key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}"
+
+struct Exception {};
+struct SafeException {};
+struct WarnException : Exception {};
+
+int functionWithThrow() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  } catch (...) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  }
+  return 0;
+}
+
+int functionWithHandling() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+return 2;
+  } catch (...) {
+return 1;
+  }
+  return 0;
+}
+
+int functionWithReThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw;
+  }
+}
+
+int functionWithNewThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw Exception();
+  }
+}
+
+void functionWithAllowedException() {
+  try {
+
+  } catch (const SafeException &) {
+  } catch (WarnException) {
+  }
+}
+
+void functionWithComment() {
+  try {
+  } catch (const Exception &) {
+// @todo: implement later, check case insensitive
+  }
+}
+
+void functionWithComment2() {
+  try {
+  } catch (const Exception &) {
+// @IGNORE: relax its safe
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-empty-catch `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
@@ -0,0 +1,149 @@
+.. title:: clang-tidy - bugprone-empty-catch
+
+bugprone-empty-catch
+
+
+Detects and suggests addressing issues with empty catch statements.
+
+.. code-block:: c++
+
+  try {
+// Some code that can throw an exception
+  } catch(const std::exception&) {
+  }
+
+Having empty catch statements in a codebase can be a serious problem that
+developers should be aware of. Catch statements are used to handle exceptions
+that are thrown during program execution. When an exception is thrown, the
+program jumps to the nearest catch statement that matches the type of the
+exception.
+
+Empty catch statements, also known as "swallowing" exceptions, catch the
+exception but do nothing with it. This means that the exception is not handled
+properly, and the program continues to run as if nothing happened. This can
+lead to several issues, such as:
+
+* *Hidden Bugs*: If an exception is caught and ignored, it can lead to hidden
+  bugs that are difficult to diagnose and fix. The root cause of the problem
+  may not be apparent, and the program may continue to behave in unexpected
+  ways.
+
+* *Security Issues*: Ignoring exceptions can lead to security issues, such as
+  buffer overflows or null pointer dereferences. Hackers can exploit these
+  vulnerabilities to gain access to sensitive data or execute malicious code.
+
+* *Poor Code Quality*: Empty catch statements can indicate poor code quality
+  and a lack of attention to detail. This can make the codebase difficult to
+  maintain and update, leading to longer development cycl

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Some release notes would be nice, looks like impacted checks are: 
performance-unnecessary-copy-initialization, 
performance-inefficient-vector-operation, performance-unnecessary-value-param, 
performance-for-range-copy. Some info about "Improved XYZ check by handling 
handling more properly const member expressions"  or something similar.  And 
wait for test results, I tried this change locally, and tests were failing.


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

https://reviews.llvm.org/D97567

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix issue preventing suppression of compiler warnings with
-Wno- under C++20 and above. Add call to
ProcessWarningOptions and propagate DiagnosticOpts more properly.

Fixes: #56709, #61969


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156056

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -23,6 +23,8 @@
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' 
%T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck 
-check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy 
-checks='-*,clang-diagnostic-*,google-explicit-constructor' 
%T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -31,6 +33,7 @@
 // CHECK3: error: unknown argument: '-fan-unknown-option' 
[clang-diagnostic-error]
 // CHECK5: error: unknown argument: '-fan-option-from-compilation-database' 
[clang-diagnostic-error]
 
+// CHECK7: :[[@LINE+4]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK2: :[[@LINE+3]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK3: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
 // CHECK5: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
@@ -43,6 +46,7 @@
 
 #define MACRO_FROM_COMMAND_LINE
 // CHECK4: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined
+// CHECK7-NOT: :[[@LINE-2]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro 
redefined
 
 #ifdef COMPILATION_ERROR
 void f(int a) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,9 @@
   be promoted to errors. For custom error promotion, use `-Werror=`
   on the compiler command-line, irrespective of `Checks` (`--checks=`) 
settings.
 
+- Fixed an issue where compiler warnings couldn't be suppressed using
+  `-Wno-` under C++20 and above.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -71,7 +71,8 @@
   InMemoryFs(new llvm::vfs::InMemoryFileSystem),
   Sources(Compiler.getSourceManager()),
   // Forward the new diagnostics to the original DiagnosticConsumer.
-  Diags(new DiagnosticIDs, new DiagnosticOptions,
+  Diags(new DiagnosticIDs,
+new DiagnosticOptions(Compiler.getDiagnosticOpts()),
 new ForwardingDiagnosticConsumer(Compiler.getDiagnosticClient())),
   LangOpts(Compiler.getLangOpts()) {
   // Add a FileSystem containing the extra files needed in place of modular
@@ -79,6 +80,7 @@
   OverlayFS->pushOverlay(InMemoryFs);
 
   Diags.setSourceManager(&Sources);
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
 
   LangOpts.Modules = false;
 


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extr

[PATCH] D78223: [clang-tidy] performance-range-for-copy only for copy.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Release notes are missing, and patch not not apply gracefully.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D78223

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

[PATCH] D126855: [clang-tidy] check_clang_tidy.py: Print output nicely in Python 3.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.
Herald added a subscriber: carlosgalvezp.

Fixed by f7a80c3d08d4821e621fc88d6a2e435291f82dff 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126855

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

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:523
 
+- Improved :doc:`performance-unnecessary-copy-initialization
+  `,

Split it multiple entry's, 1 per check, keep in alphabetical order. 
and maybe keep it in format;
"Improved XYZ check by extending const usage analysis to include the type's 
members."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:523
 
+- Improved :doc:`performance-unnecessary-copy-initialization
+  `,

PiotrZSL wrote:
> Split it multiple entry's, 1 per check, keep in alphabetical order. 
> and maybe keep it in format;
> "Improved XYZ check by extending const usage analysis to include the type's 
> members."
And if we already got entry for some check, just try adding this info to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D78223: [clang-tidy] performance-range-for-copy only for copy.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp:66-67
   }
+  for (const S S2 : C) {
+  }
 }

Those tests already exist and pass in current version.
Most probably this change is obsolete.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78223

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:83
   Diags.setSourceManager(&Sources);
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
 

carlosgalvezp wrote:
> A bit unclear to me why we should add this line here, grepping for this 
> function in the repo I only find hits in the `clang` folder. How come it's 
> not needed in other places?
We create here new Preprocessor (line 96) and new DiagEngine (line 74), when 
C++20/Modules are enabled this class is register as an second Preprocessor and 
both are (+-) executed.
Unfortunately when we pass `-Wno-macro-redefined` it's pass only to original 
DiagEngine, and we run into situation when warning is suppressed by first 
DiagEngine, but not by second that is used by second Preprocessor. 

Passing DiagnosticOptions alone to DiagEngine looks to be insufficient, as it's 
does not apply settings, only calling this function apply them. (somehow).
This is gray area for me.

More about problem here: 
https://discourse.llvm.org/t/rfc-expand-modular-headers-ppcallbacks-problem-in-c-20/71628


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

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

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:523
 
+- Improved :doc:`performance-for-range-copy
+  `

sort them by name, performance checks are before readability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf256fee53430: [clang-tidy] Add bugprone-empty-catch check 
(authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:{key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}"
+
+struct Exception {};
+struct SafeException {};
+struct WarnException : Exception {};
+
+int functionWithThrow() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  } catch (...) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  }
+  return 0;
+}
+
+int functionWithHandling() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+return 2;
+  } catch (...) {
+return 1;
+  }
+  return 0;
+}
+
+int functionWithReThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw;
+  }
+}
+
+int functionWithNewThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw Exception();
+  }
+}
+
+void functionWithAllowedException() {
+  try {
+
+  } catch (const SafeException &) {
+  } catch (WarnException) {
+  }
+}
+
+void functionWithComment() {
+  try {
+  } catch (const Exception &) {
+// @todo: implement later, check case insensitive
+  }
+}
+
+void functionWithComment2() {
+  try {
+  } catch (const Exception &) {
+// @IGNORE: relax its safe
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-empty-catch `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
@@ -0,0 +1,149 @@
+.. title:: clang-tidy - bugprone-empty-catch
+
+bugprone-empty-catch
+
+
+Detects and suggests addressing issues with empty catch statements.
+
+.. code-block:: c++
+
+  try {
+// Some code that can throw an exception
+  } catch(const std::exception&) {
+  }
+
+Having empty catch statements in a codebase can be a serious problem that
+developers should be aware of. Catch statements are used to handle exceptions
+that are thrown during program execution. When an exception is thrown, the
+program jumps to the nearest catch statement that matches the type of the
+exception.
+
+Empty catch statements, also known as "swallowing" exceptions, catch the
+exception but do nothing with it. This means that the exception is not handled
+properly, and the program continues to run as if nothing happened. This can
+lead to several issues, such as:
+
+* *Hidden Bugs*: If an exception is caught and ignored, it can lead to hidden
+  bugs that are difficult to diagnose and fix. The root cause of the problem
+  may not be apparent, and the program may continue to behave in unexpected
+  ways.
+
+* *Security Issues*: Ignoring exceptions can lead to security issues, such as
+  buffer overflows or null pointer dereferences. Hackers can exploit these
+  vulnerabilities to gain access to sensitive data or execute malicious code.
+
+* *Poor Code Quality*: Empty catch statements can indicate poor code quality
+  and a lack of attention to detail. This can make the codebase difficult to
+  maintain and update,

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543314.
PiotrZSL added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Add -fexceptions to tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
@@ -27,6 +27,7 @@
 "DanglingHandleCheck.cpp",
 "DynamicStaticInitializersCheck.cpp",
 "EasilySwappableParametersCheck.cpp",
+"EmptyCatchCheck.cpp",
 "ExceptionEscapeCheck.cpp",
 "FoldInitTypeCheck.cpp",
 "ForwardDeclarationNamespaceCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:{key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}" -- -fexceptions
+
+struct Exception {};
+struct SafeException {};
+struct WarnException : Exception {};
+
+int functionWithThrow() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  } catch (...) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  }
+  return 0;
+}
+
+int functionWithHandling() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+return 2;
+  } catch (...) {
+return 1;
+  }
+  return 0;
+}
+
+int functionWithReThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw;
+  }
+}
+
+int functionWithNewThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw Exception();
+  }
+}
+
+void functionWithAllowedException() {
+  try {
+
+  } catch (const SafeException &) {
+  } catch (WarnException) {
+  }
+}
+
+void functionWithComment() {
+  try {
+  } catch (const Exception &) {
+// @todo: implement later, check case insensitive
+  }
+}
+
+void functionWithComment2() {
+  try {
+  } catch (const Exception &) {
+// @IGNORE: relax its safe
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-empty-catch `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
@@ -0,0 +1,149 @@
+.. title:: clang-tidy - bugprone-empty-catch
+
+bugprone-empty-catch
+
+
+Detects and suggests addressing issues with empty catch statements.
+
+.. code-block:: c++
+
+  try {
+// Some code that can throw an exception
+  } catch(const std::exception&) {
+  }
+
+Having empty catch statements in a codebase can be a serious problem that
+developers should be aware of. Catch statements are used to handle exceptions
+that are thrown during program execution. When an exception is thrown, the
+program jumps to the nearest catch statement that matches the type of the
+exception.
+
+Empty catch statements, also known as "swallowing" exceptions, catch the
+exception but do nothing with it. This means that the exception is not handled
+properly, and the program continues to run as if nothing happened. This can
+lead to several issues, such as:
+
+* *Hid

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75
+  Diags(new DiagnosticIDs,
+new DiagnosticOptions(Compiler.getDiagnosticOpts()),
 new ForwardingDiagnosticConsumer(Compiler.getDiagnosticClient())),

carlosgalvezp wrote:
> When downloading your patch, this seems to not be needed to make the tests 
> pass, should it be removed?
No idea, it seem reasonable.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:83
   Diags.setSourceManager(&Sources);
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
 

carlosgalvezp wrote:
> PiotrZSL wrote:
> > carlosgalvezp wrote:
> > > A bit unclear to me why we should add this line here, grepping for this 
> > > function in the repo I only find hits in the `clang` folder. How come 
> > > it's not needed in other places?
> > We create here new Preprocessor (line 96) and new DiagEngine (line 74), 
> > when C++20/Modules are enabled this class is register as an second 
> > Preprocessor and both are (+-) executed.
> > Unfortunately when we pass `-Wno-macro-redefined` it's pass only to 
> > original DiagEngine, and we run into situation when warning is suppressed 
> > by first DiagEngine, but not by second that is used by second Preprocessor. 
> > 
> > Passing DiagnosticOptions alone to DiagEngine looks to be insufficient, as 
> > it's does not apply settings, only calling this function apply them. 
> > (somehow).
> > This is gray area for me.
> > 
> > More about problem here: 
> > https://discourse.llvm.org/t/rfc-expand-modular-headers-ppcallbacks-problem-in-c-20/71628
> Thanks for the explanation! I'm not sure what the best way forward is. Would 
> it make sense to add some `TODO` or `FIXME` comment to further investigate in 
> the future if we want that line of code ?
Yes, FIXME could be added, but ExpandModularHeadersPPCallbacks got also other 
issues, for example with TargetTriple propagation and __has_builtin, looks like 
all those got same source, simply we shoudn't create separate Preprocessor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

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

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG047273fc9c84: [clang-tidy] Add bugprone-empty-catch check 
(authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/bugprone/BUILD.gn
@@ -27,6 +27,7 @@
 "DanglingHandleCheck.cpp",
 "DynamicStaticInitializersCheck.cpp",
 "EasilySwappableParametersCheck.cpp",
+"EmptyCatchCheck.cpp",
 "ExceptionEscapeCheck.cpp",
 "FoldInitTypeCheck.cpp",
 "ForwardDeclarationNamespaceCheck.cpp",
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/empty-catch.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-empty-catch %t -- \
+// RUN: -config="{CheckOptions: [{key: bugprone-empty-catch.AllowEmptyCatchForExceptions, value: '::SafeException;WarnException'}, \
+// RUN:{key: bugprone-empty-catch.IgnoreCatchWithKeywords, value: '@IGNORE;@TODO'}]}" -- -fexceptions
+
+struct Exception {};
+struct SafeException {};
+struct WarnException : Exception {};
+
+int functionWithThrow() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  } catch (...) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: empty catch statements hide issues; to handle exceptions appropriately, consider re-throwing, handling, or avoiding catch altogether [bugprone-empty-catch]
+  }
+  return 0;
+}
+
+int functionWithHandling() {
+  try {
+throw 5;
+  } catch (const Exception &) {
+return 2;
+  } catch (...) {
+return 1;
+  }
+  return 0;
+}
+
+int functionWithReThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw;
+  }
+}
+
+int functionWithNewThrow() {
+  try {
+throw 5;
+  } catch (...) {
+throw Exception();
+  }
+}
+
+void functionWithAllowedException() {
+  try {
+
+  } catch (const SafeException &) {
+  } catch (WarnException) {
+  }
+}
+
+void functionWithComment() {
+  try {
+  } catch (const Exception &) {
+// @todo: implement later, check case insensitive
+  }
+}
+
+void functionWithComment2() {
+  try {
+  } catch (const Exception &) {
+// @IGNORE: relax its safe
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
`bugprone-dangling-handle `_,
`bugprone-dynamic-static-initializers `_,
`bugprone-easily-swappable-parameters `_,
+   `bugprone-empty-catch `_,
`bugprone-exception-escape `_,
`bugprone-fold-init-type `_,
`bugprone-forward-declaration-namespace `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst
@@ -0,0 +1,149 @@
+.. title:: clang-tidy - bugprone-empty-catch
+
+bugprone-empty-catch
+
+
+Detects and suggests addressing issues with empty catch statements.
+
+.. code-block:: c++
+
+  try {
+// Some code that can throw an exception
+  } catch(const std::exception&) {
+  }
+
+Having empty catch statements in a codebase can be a serious problem that
+developers should be aware of. Catch statements are used to handle exceptions
+that are thrown during program execution. When an exception is thrown, the
+program jumps to the nearest catch statement that matches the type of the
+exception.
+
+Empty catch statements, also known as "swallowing" exceptions, catch the
+exception but do nothing with it. This means that the exception is not handled
+properly, and the program continues to run as if nothing ha

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked an inline comment as done.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75
+  Diags(new DiagnosticIDs,
+new DiagnosticOptions(Compiler.getDiagnosticOpts()),
 new ForwardingDiagnosticConsumer(Compiler.getDiagnosticClient())),

carlosgalvezp wrote:
> PiotrZSL wrote:
> > carlosgalvezp wrote:
> > > When downloading your patch, this seems to not be needed to make the 
> > > tests pass, should it be removed?
> > No idea, it seem reasonable.
> Do you mean it seems reasonable to keep it, or reasonable to remove it?
reasonable to keep it, we want both DiagEngines to have same settings


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked an inline comment as done.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75
+  Diags(new DiagnosticIDs,
+new DiagnosticOptions(Compiler.getDiagnosticOpts()),
 new ForwardingDiagnosticConsumer(Compiler.getDiagnosticClient())),

carlosgalvezp wrote:
> PiotrZSL wrote:
> > carlosgalvezp wrote:
> > > PiotrZSL wrote:
> > > > carlosgalvezp wrote:
> > > > > When downloading your patch, this seems to not be needed to make the 
> > > > > tests pass, should it be removed?
> > > > No idea, it seem reasonable.
> > > Do you mean it seems reasonable to keep it, or reasonable to remove it?
> > reasonable to keep it, we want both DiagEngines to have same settings
> Reason I ask is that it seems the majority of `DiagnosticOptions` are 
> initialized with default ctor:
> 
> ```
> $ git grep -E " DiagnosticOptions\(\w" | wc -l
> 3
> $ git grep -E " DiagnosticOptions\(\)" | wc -l
> 74
> ```
> 
> > we want both DiagEngines to have same settings
> 
> Do you know where "the other" `DiagEngine` is initialized? The one I can find 
> is initialized without the compiler opts.
> 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidy.cpp#L544
> 
> Since the added code does not have any impact on the result of the test, I'd 
> argue that either the test is insufficient or the added code is dead code 
> that should be removed.
sure, maybe ProcessWarningOptions will override it anyway, I didnt check more 
deeply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked an inline comment as done.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75
+  Diags(new DiagnosticIDs,
+new DiagnosticOptions(Compiler.getDiagnosticOpts()),
 new ForwardingDiagnosticConsumer(Compiler.getDiagnosticClient())),

carlosgalvezp wrote:
> PiotrZSL wrote:
> > carlosgalvezp wrote:
> > > PiotrZSL wrote:
> > > > carlosgalvezp wrote:
> > > > > PiotrZSL wrote:
> > > > > > carlosgalvezp wrote:
> > > > > > > When downloading your patch, this seems to not be needed to make 
> > > > > > > the tests pass, should it be removed?
> > > > > > No idea, it seem reasonable.
> > > > > Do you mean it seems reasonable to keep it, or reasonable to remove 
> > > > > it?
> > > > reasonable to keep it, we want both DiagEngines to have same settings
> > > Reason I ask is that it seems the majority of `DiagnosticOptions` are 
> > > initialized with default ctor:
> > > 
> > > ```
> > > $ git grep -E " DiagnosticOptions\(\w" | wc -l
> > > 3
> > > $ git grep -E " DiagnosticOptions\(\)" | wc -l
> > > 74
> > > ```
> > > 
> > > > we want both DiagEngines to have same settings
> > > 
> > > Do you know where "the other" `DiagEngine` is initialized? The one I can 
> > > find is initialized without the compiler opts.
> > > 
> > > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidy.cpp#L544
> > > 
> > > Since the added code does not have any impact on the result of the test, 
> > > I'd argue that either the test is insufficient or the added code is dead 
> > > code that should be removed.
> > sure, maybe ProcessWarningOptions will override it anyway, I didnt check 
> > more deeply.
> In that case, could you please revert the change to this line, since it's not 
> needed?
Yes, but later today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543601.
PiotrZSL marked 3 inline comments as done.
PiotrZSL added a comment.

Review comments fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -23,6 +23,8 @@
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' 
%T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck 
-check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy 
-checks='-*,clang-diagnostic-*,google-explicit-constructor' 
%T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -31,6 +33,7 @@
 // CHECK3: error: unknown argument: '-fan-unknown-option' 
[clang-diagnostic-error]
 // CHECK5: error: unknown argument: '-fan-option-from-compilation-database' 
[clang-diagnostic-error]
 
+// CHECK7: :[[@LINE+4]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK2: :[[@LINE+3]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK3: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
 // CHECK5: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
@@ -43,6 +46,7 @@
 
 #define MACRO_FROM_COMMAND_LINE
 // CHECK4: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined
+// CHECK7-NOT: :[[@LINE-2]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro 
redefined
 
 #ifdef COMPILATION_ERROR
 void f(int a) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,9 @@
   be promoted to errors. For custom error promotion, use `-Werror=`
   on the compiler command-line, irrespective of `Checks` (`--checks=`) 
settings.
 
+- Fixed an issue where compiler warnings couldn't be suppressed using
+  `-Wno-` under C++20 and above.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -79,6 +79,9 @@
   OverlayFS->pushOverlay(InMemoryFs);
 
   Diags.setSourceManager(&Sources);
+  // FIXME: Investigate whatever is there better way to initialize DiagEngine
+  // or whatever DiagEngine can be shared by multiple preprocessors
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
 
   LangOpts.Modules = false;
 


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -23,6 +23,8 @@
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck -check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %

[PATCH] D156024: [clang-tidy] Add --experimental-disable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543641.
PiotrZSL retitled this revision from "[clang-tidy] Add 
--disable-modular-headers-expansion option" to "[clang-tidy] Add 
--experimental-disable-module-headers-parsing option".
PiotrZSL added a comment.

Rename of option + cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156024

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp


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
@@ -263,6 +263,11 @@
cl::init(false), cl::Hidden,
cl::cat(ClangTidyCategory));
 
+static cl::opt
+DisableModuleHeadersParsing("experimental-disable-module-headers-parsing",
+cl::init(false), cl::Hidden,
+cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -659,7 +664,8 @@
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
-   AllowEnablingAnalyzerAlphaCheckers);
+   AllowEnablingAnalyzerAlphaCheckers,
+   DisableModuleHeadersParsing);
   std::vector Errors =
   runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
FixNotes, EnableCheckProfile, ProfilePrefix);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -70,7 +70,8 @@
 public:
   /// Initializes \c ClangTidyContext instance.
   ClangTidyContext(std::unique_ptr OptionsProvider,
-   bool AllowEnablingAnalyzerAlphaCheckers = false);
+   bool AllowEnablingAnalyzerAlphaCheckers = false,
+   bool DisableModuleHeadersParsing = false);
   /// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
   // FIXME: this is required initialization, and should be a constructor param.
   // Fix the context -> diag engine -> consumer -> context initialization 
cycle.
@@ -198,6 +199,10 @@
 return AllowEnablingAnalyzerAlphaCheckers;
   }
 
+  bool canEnableModuleHeadersParsing() const {
+return !DisableModuleHeadersParsing;
+  }
+
   void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
 
   bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -245,6 +250,7 @@
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool DisableModuleHeadersParsing;
 
   bool SelfContainedDiags;
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -161,10 +161,11 @@
 
 ClangTidyContext::ClangTidyContext(
 std::unique_ptr OptionsProvider,
-bool AllowEnablingAnalyzerAlphaCheckers)
+bool AllowEnablingAnalyzerAlphaCheckers, bool DisableModuleHeadersParsing)
 : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
   Profile(false),
   AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+  DisableModuleHeadersParsing(DisableModuleHeadersParsing),
   SelfContainedDiags(false) {
   // Before the first translation unit we can get errors related to 
command-line
   // parsing, use empty string for the file name in this case.
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -418,7 +418,8 @@
   Preprocessor *PP = &Compiler.getPreprocessor();
   Preprocessor *ModuleExpanderPP = PP;
 
-  if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+  if (Context.canEnableModuleHeadersParsing() &&
+  Context.getLangOpts().Modules && OverlayFS != nullptr) {
 auto ModuleExpander = std::make_unique(
 &Compiler, OverlayFS);
 ModuleExpanderPP = ModuleExpander->getPreprocessor();


Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cp

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Some entry in release notes or/and check documentation would be welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890

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

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst:11
 This check identifies unsafe accesses to values contained in
-``std::optional``, ``absl::optional``, or ``base::Optional``
+``std::optional``, ``absl::optional``, ``base::Optional``, or 
``folly::Optional``
 objects. Below we will refer to all these types collectively as 
``optional``.

80 characters limit, wrap it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890

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

[PATCH] D156161: [clang-tidy] Add --experimental-enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Change default behavior in Clang-tidy and skip by default
module headers parsing. That functionality is buggy and
slow in C++20, and provide tiny benefit.

Depends on D156024 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156161

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
@@ -5,7 +5,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --experimental-enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // RUN: rm -rf %t
@@ -14,7 +14,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --experimental-enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // Run clang-tidy on a file with modular includes:
@@ -25,7 +25,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --experimental-enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
@@ -37,7 +37,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --experimental-enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,20 @@
   be promoted to errors. For custom error promotion, use `-Werror=`
   on the compiler command-line, irrespective of `Checks` (`--checks=`) settings.
 
+- Preprocessor-level module header parsing is now disabled by default due to
+  the problems it caused in C++20 and above, leading to performance and code
+  parsing issues regardless of whether modules were used or not. This change
+  will impact only the following checks:
+  :doc:`modernize-replace-disallow-copy-and-assign-macro
+  `,
+  :doc:`bugprone-reserved-identifier
+  `, and
+  :doc:`readability-identifier-naming
+  `. Those checks will no
+  longer see macros defined in modules. Users can still enable this
+  functionality using the hidden command line option
+  `--experimental-enable-module-headers-parsing`.
+
 New checks
 ^^
 
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
@@ -264,9 +264,9 @@
cl::cat(ClangTidyCategory));
 
 static cl::opt
-DisableModuleHeadersParsing("experimental-disable-module-headers-parsing",
-cl::init(false), cl::Hidden,
-cl::cat(ClangTidyCategory));
+EnableModuleHeadersParsing("experimental-enable-module-headers-parsing",
+   cl::init(false), cl::Hidden,
+   cl::cat(ClangTidyCategory));
 
 static cl::opt ExportFixes("export-fixes", desc(R"(
 YAML file to store suggested fixes in. The
@@ -665,7 +665,7 @@
 
   ClangTidyContext Cont

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.

+- LGTM, I will correct those nits and push it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890

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

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

[Piotr Zegar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f0630f8bc91: [clang-tidy] Add folly::Optional to 
unchecked-optional-access (authored by Anton Dukeman 
, committed by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D155890?vs=543659&id=543678#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/folly/types/Optional.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -56,9 +56,10 @@
   }
 
   if (RD.getName() == "Optional") {
-// Check whether namespace is "::base".
+// Check whether namespace is "::base" or "::folly".
 const auto *N = dyn_cast_or_null(RD.getDeclContext());
-return N != nullptr && isTopLevelNamespaceWithName(*N, "base");
+return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") ||
+isTopLevelNamespaceWithName(*N, "folly"));
   }
 
   return false;
@@ -87,8 +88,8 @@
 /// Matches any of the spellings of the optional types and sugar, aliases, etc.
 auto hasOptionalType() { return hasType(optionalOrAliasType()); }
 
-auto isOptionalMemberCallWithName(
-llvm::StringRef MemberName,
+auto isOptionalMemberCallWithNameMatcher(
+ast_matchers::internal::Matcher matcher,
 const std::optional &Ignorable = std::nullopt) {
   auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))
 : cxxThisExpr());
@@ -96,7 +97,7 @@
   on(expr(Exception,
   anyOf(hasOptionalType(),
 hasType(pointerType(pointee(optionalOrAliasType())),
-  callee(cxxMethodDecl(hasName(MemberName;
+  callee(cxxMethodDecl(matcher)));
 }
 
 auto isOptionalOperatorCallWithName(
@@ -109,15 +110,15 @@
 }
 
 auto isMakeOptionalCall() {
-  return callExpr(
-  callee(functionDecl(hasAnyName(
-  "std::make_optional", "base::make_optional", "absl::make_optional"))),
-  hasOptionalType());
+  return callExpr(callee(functionDecl(hasAnyName(
+  "std::make_optional", "base::make_optional",
+  "absl::make_optional", "folly::make_optional"))),
+  hasOptionalType());
 }
 
 auto nulloptTypeDecl() {
-  return namedDecl(
-  hasAnyName("std::nullopt_t", "absl::nullopt_t", "base::nullopt_t"));
+  return namedDecl(hasAnyName("std::nullopt_t", "absl::nullopt_t",
+  "base::nullopt_t", "folly::None"));
 }
 
 auto hasNulloptType() { return hasType(nulloptTypeDecl()); }
@@ -129,8 +130,8 @@
 }
 
 auto inPlaceClass() {
-  return recordDecl(
-  hasAnyName("std::in_place_t", "absl::in_place_t", "base::in_place_t"));
+  return recordDecl(hasAnyName("std::in_place_t", "absl::in_place_t",
+   "base::in_place_t", "folly::in_place_t"));
 }
 
 auto isOptionalNulloptConstructor() {
@@ -750,7 +751,8 @@
 
 StatementMatcher
 valueCall(const std::optional &IgnorableOptional) {
-  return isOptionalMemberCallWithName("value", IgnorableOptional);
+  return isOptionalMemberCallWithNameMatcher(hasName("value"),
+ IgnorableOptional);
 }
 
 StatementMatcher
@@ -832,19 +834,22 @@
  transferArrowOpCall(E, E->getArg(0), State);
})
 
-  // optional::has_value
+  // optional::has_value, optional::hasValue
+  // Of the supported optionals only folly::Optional uses hasValue, but this
+  // will also pass for other types
   .CaseOfCFGStmt(
-  isOptionalMemberCallWithName("has_value"),
+  isOptionalMemberCallWithNameMatcher(
+  hasAnyName("has_value", "hasValue")),
   transferOptionalHasValueCall)
 
   // optional::operator bool
   .CaseOfCFGStmt(
-  isOptionalMemberCallWithName("operator bool"),
+  isOptionalMemberCallWithNameMatcher(hasName("operator bool")),
   transferOptionalHasValueCall)
 
   // optional::emplace
   .CaseOfCFGStmt(
-  isOptionalMemberCallWithName("emplace"),
+  isOptionalMemberCallWithNameMatcher(hasName("emplace")),
   [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
  

[PATCH] D156161: [clang-tidy] Add --experimental-enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

@carlosgalvezp
My plan was to push first change regardless (so they would be some workaround, 
even if hidden & undocumented), and second only if approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156161

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

[PATCH] D156024: [clang-tidy] Add --experimental-disable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:202
 
+  bool canEnableModuleHeadersParsing() const {
+return !DisableModuleHeadersParsing;

carlosgalvezp wrote:
> Add docs?
for function ? It's straight forward, but yes, I can add some one liner...



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:266
 
+static cl::opt
+DisableModuleHeadersParsing("experimental-disable-module-headers-parsing",

carlosgalvezp wrote:
> Add documentation
This is hidden option. allow-enabling-analyzer-alpha-checkers is also hidden 
and has no documentation. That I base on.
So 2 questions:
- should it be hidden ?
- if stay hidden, does it need documentation ? (I'm fine with adding some)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156024

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

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

[Piotr Zegar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG992fa7be3438: [clang-tidy] Initialize DiagnosticEngine in 
ExpandModularHeaders (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156056

Files:
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -23,6 +23,8 @@
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' 
%T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck 
-check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy 
-checks='-*,clang-diagnostic-*,google-explicit-constructor' 
%T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -31,6 +33,7 @@
 // CHECK3: error: unknown argument: '-fan-unknown-option' 
[clang-diagnostic-error]
 // CHECK5: error: unknown argument: '-fan-option-from-compilation-database' 
[clang-diagnostic-error]
 
+// CHECK7: :[[@LINE+4]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK2: :[[@LINE+3]]:9: warning: implicit conversion from 'double' to 'int' 
changes value from 1.5 to 1 [clang-diagnostic-literal-conversion]
 // CHECK3: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
 // CHECK5: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' 
changes value
@@ -43,6 +46,7 @@
 
 #define MACRO_FROM_COMMAND_LINE
 // CHECK4: :[[@LINE-1]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro redefined
+// CHECK7-NOT: :[[@LINE-2]]:9: warning: 'MACRO_FROM_COMMAND_LINE' macro 
redefined
 
 #ifdef COMPILATION_ERROR
 void f(int a) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,9 @@
   be promoted to errors. For custom error promotion, use `-Werror=`
   on the compiler command-line, irrespective of `Checks` (`--checks=`) 
settings.
 
+- Fixed an issue where compiler warnings couldn't be suppressed using
+  `-Wno-` under C++20 and above.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
===
--- clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -79,6 +79,9 @@
   OverlayFS->pushOverlay(InMemoryFs);
 
   Diags.setSourceManager(&Sources);
+  // FIXME: Investigate whatever is there better way to initialize DiagEngine
+  // or whatever DiagEngine can be shared by multiple preprocessors
+  ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts());
 
   LangOpts.Modules = false;
 


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -23,6 +23,8 @@
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %T/diagnostics/input.cpp -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %T/diagnostics/input.cpp 2>&1 | FileCheck -check-prefix=CHECK5 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: not clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/i

[PATCH] D156024: [clang-tidy] Add --experimental-disable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL abandoned this revision.
PiotrZSL added a comment.

Will be integrated into D156161 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156024

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

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 543708.
PiotrZSL retitled this revision from "[clang-tidy] Add 
--experimental-enable-module-headers-parsing option" to "[clang-tidy] Add 
--enable-module-headers-parsing option".
PiotrZSL edited the summary of this revision.
PiotrZSL added a comment.
Herald added a subscriber: arphaman.

Merge with other change, remove experimental


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156161

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
@@ -5,7 +5,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // RUN: rm -rf %t
@@ -14,7 +14,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // Run clang-tidy on a file with modular includes:
@@ -25,7 +25,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
@@ -37,7 +37,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -116,117 +116,120 @@
 
   Generic Options:
 
---help - Display available options (--help-hidden for more)
---help-list- Display list of available options (--help-list-hidden for more)
---version  - Display the version of this program
+--help  - Display available options (--help-hidden for more)
+--help-list - Display list of available options (--help-list-hidden for more)
+--version   - Display the version of this program
 
   clang-tidy options:
 
---checks=  - Comma-separated list of globs with optional '-'
- prefix. Globs are processed in order of
- appearance in the list. Globs without '-'
- prefix add checks with matching names to the
- set, globs with the '-' prefix remove checks
- with matching names from the set of enabled
- checks. This option's value is appended to the
- value of the 'Checks' option in .clang-tidy
- file, if any.
---config=  - Specifies a configuration in YAML/JSON format:
-   -config="{Checks: '*',
- CheckOptions: {x: y}}"
- When the value is empty, clang-tidy will
- attempt to find a file named .clang-tidy for
- each source file in its parent

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:266
 
+static cl::opt 
EnableModuleHeadersParsing("enable-module-headers-parsing",
+desc(R"(

carlosgalvezp wrote:
> --experimental-enable-module-headers-parsing
I removed 'experimental' because it were messing with the --help output, were 
causing it be very wide. 
And it's no longer hidden option now. so should it be hidden & experimental ? 
Or non hidden...



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:270
+for C++20 and above, empowering specific checks
+to detect macro definitions within modules.
+)"),

carlosgalvezp wrote:
> Should we document the implications/risks of enabling this, so people are 
> informed? Also the fact that is experimental and subject to change.
Something like "May cause performance & parsing issues, and therefore is 
considered experimental." ? I'ts fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156161

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:39
+  ID) {
+  NotExtendedByDeclBoundToPredicate Predicate;
+  Predicate.ID = ID;

xgupta wrote:
> Can be written using direct initialization as  
> `NotExtendedByDeclBoundToPredicate Predicate{ID};`
Yes, but I followed like others are done.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18
+
+   const std::string& value("hello");
+

xgupta wrote:
> The below comment is not matching,  do you want to write - 
> `const std::string& str = std::string("hello");`
> ?
actually comment is +- fine, constructor to std::string will be called anyway, 
and for compiler I think those 2 are equal.
Will verify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 544034.
PiotrZSL added a comment.

Fix doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156161

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
@@ -5,7 +5,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // RUN: rm -rf %t
@@ -14,7 +14,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // Run clang-tidy on a file with modular includes:
@@ -25,7 +25,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
@@ -37,7 +37,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -116,117 +116,122 @@
 
   Generic Options:
 
---help - Display available options (--help-hidden for more)
---help-list- Display list of available options (--help-list-hidden for more)
---version  - Display the version of this program
+--help  - Display available options (--help-hidden for more)
+--help-list - Display list of available options (--help-list-hidden for more)
+--version   - Display the version of this program
 
   clang-tidy options:
 
---checks=  - Comma-separated list of globs with optional '-'
- prefix. Globs are processed in order of
- appearance in the list. Globs without '-'
- prefix add checks with matching names to the
- set, globs with the '-' prefix remove checks
- with matching names from the set of enabled
- checks. This option's value is appended to the
- value of the 'Checks' option in .clang-tidy
- file, if any.
---config=  - Specifies a configuration in YAML/JSON format:
-   -config="{Checks: '*',
- CheckOptions: {x: y}}"
- When the value is empty, clang-tidy will
- attempt to find a file named .clang-tidy for
- each source file in its parent directories.
---config-file= - Specify the path of .clang-tidy or custom config file:
- e.g. --config-file=/some/path/myTidyConfigFile
- This option internally works exactly the same way as
-

[PATCH] D144135: [clang-tidy] Add performance-enum-size check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3397dae69e09: [clang-tidy] Add performance-enum-size check 
(authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D144135?vs=543196&id=544042#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.cpp
  clang-tools-extra/clang-tidy/performance/EnumSizeCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
  clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/enum-size.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s performance-enum-size %t -- \
+// RUN:   -config="{CheckOptions: [{key: performance-enum-size.EnumIgnoreList, value: '::IgnoredEnum;IgnoredSecondEnum'}]}"
+
+namespace std
+{
+using uint8_t = unsigned char;
+using int8_t = signed char;
+using uint16_t = unsigned short;
+using int16_t = signed short;
+using uint32_t = unsigned int;
+using int32_t = signed int;
+}
+
+enum class Value
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'Value' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported
+};
+
+
+enum class EnumClass : std::int16_t
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: enum 'EnumClass' uses a larger base type ('std::int16_t' (aka 'short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported
+};
+
+enum EnumWithType : std::uint16_t
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithType' uses a larger base type ('std::uint16_t' (aka 'unsigned short'), size: 2 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+supported,
+supported2
+};
+
+enum EnumWithNegative
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumWithNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+s1 = -128,
+s2 = -100,
+s3 = 100,
+s4 = 127
+};
+
+enum EnumThatCanBeReducedTo2Bytes
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumThatCanBeReducedTo2Bytes' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+a1 = -128,
+a2 = 0x6EEE
+};
+
+enum EnumOnlyNegative
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'EnumOnlyNegative' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]
+{
+b1 = -125,
+b2 = -50,
+b3 = -10
+};
+
+enum CorrectU8 : std::uint8_t
+{
+c01 = 10,
+c02 = 11
+};
+
+enum CorrectU16 : std::uint16_t
+{
+c11 = 10,
+c12 = 0x
+};
+
+constexpr int getValue()
+{
+return 256;
+}
+
+
+enum CalculatedDueToUnknown1 : unsigned int
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown1' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+c21 = 10,
+c22 = getValue()
+};
+
+enum CalculatedDueToUnknown2 : unsigned int
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: enum 'CalculatedDueToUnknown2' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint16_t' (2 bytes) as the base type to reduce its size [performance-enum-size]
+{
+c31 = 10,
+c32 = c31 + 246
+};
+
+enum class IgnoredEnum : std::uint32_t
+{
+unused1 = 1,
+unused2 = 2
+};
+
+namespace internal
+{
+
+enum class IgnoredSecondEnum
+{
+unused1 = 1,
+unused2 = 2
+};
+
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance/enum-size.rst
@@ -0,0 +1,72 @@
+.. title:: clang-tidy - performance-enum-size
+
+performance-enum-size
+=
+
+Recommends the smallest possible

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

[Piotr Zegar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb530eeea5e76: [clang-tidy] Add 
--enable-module-headers-parsing option (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156161

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
@@ -5,7 +5,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // RUN: rm -rf %t
@@ -14,7 +14,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/without-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/
 //
 // Run clang-tidy on a file with modular includes:
@@ -25,7 +25,7 @@
 // RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
@@ -37,7 +37,7 @@
 // RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/with-modules -- \
 // RUN:   -config="CheckOptions: [{ \
 // RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN:   -header-filter=.* \
+// RUN:   -header-filter=.* --enable-module-headers-parsing \
 // RUN:   -- -I %t/ \
 // RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
 // RUN:   -fmodule-map-file=%t/module.modulemap \
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -116,117 +116,122 @@
 
   Generic Options:
 
---help - Display available options (--help-hidden for more)
---help-list- Display list of available options (--help-list-hidden for more)
---version  - Display the version of this program
+--help  - Display available options (--help-hidden for more)
+--help-list - Display list of available options (--help-list-hidden for more)
+--version   - Display the version of this program
 
   clang-tidy options:
 
---checks=  - Comma-separated list of globs with optional '-'
- prefix. Globs are processed in order of
- appearance in the list. Globs without '-'
- prefix add checks with matching names to the
- set, globs with the '-' prefix remove checks
- with matching names from the set of enabled
- checks. This option's value is appended to the
- value of the 'Checks' option in .clang-tidy
- file, if any.
---config=  - Specifies a configuration in YAML/JSON format:
-   -config="{Checks: '*',
- CheckOptions: {x: y}}"
- When the value is empty, clang-tidy will
- attempt to find a file named .clang-tidy for
- each source file in its parent directories.
---config-file= - Specify the path of .clang-tidy or custom config file:
-   

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:10
+This construction is often the result of multiple code refactorings or a lack
+of developer knowledge, leading to confusion or subtle bugs. In most cases,
+what the developer really wanted to do is create a new variable rather than

xgupta wrote:
> May be good to mention dangling references and resource leakage as potential 
> issues.
There will be no dangling references and resource leakage because lifetime of 
temporary is extended by variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 544057.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-reference-to-constructed-temporary %t
+
+struct WithConstructor
+{
+WithConstructor(int, int);
+};
+
+struct WithoutConstructor
+{
+int a, b;
+};
+
+void test()
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: reference variable 'tmp1' extends the lifetime of a just-constructed temporary object 'const WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithConstructor& tmp1{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: reference variable 'tmp2' extends the lifetime of a just-constructed temporary object 'const WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithoutConstructor& tmp2{1,2};
+
+
+// CHECK-MESSAGES: :[[@LINE+1]]:22: warning: reference variable 'tmp3' extends the lifetime of a just-constructed temporary object 'WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithConstructor&& tmp3{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: reference variable 'tmp4' extends the lifetime of a just-constructed temporary object 'WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithoutConstructor&& tmp4{1,2};
+
+   WithConstructor tmp5{1,2};
+   WithoutConstructor tmp6{1,2};
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-reference-to-constructed-temporary
+
+readability-reference-to-constructed-temporary
+==
+
+Detects C++ code where a reference variable is used to extend the lifetime of
+a temporary object that has just been constructed.
+
+This construction is often the result of multiple code refactorings or a lack
+of developer knowledge, leading to confusion or subtle bugs. In most cases,
+what the developer really wanted to do is create a new variable rather than
+extending the lifetime of a temporary object.
+
+Examples of problematic code include:
+
+.. code-block:: c++
+
+   const std::string& value("hello");
+
+   struct Point { int x; int y; };
+   const Point& p = { 1, 2 };
+
+In the first example, a ``const std::string&`` reference variable ``str`` is
+assigned a temporary object created by the ``std::string("hello")``
+constructor. In the second example, a ``const Point&`` reference variable ``p``
+is assigned an object that is constructed from an initializer list ``{ 1, 2 }``.
+Both of these examples extend the lifetime of the temporary object to the
+lifetime of the reference variable, which can make it difficult to reason about
+and may lead to subtle bugs or misunderstanding.
+
+To avoid these issues, it is recommended to change the reference variable to a
+(``const``) value variable.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -379,6 +379,7 @@
`readability-redundant-smartptr-get `_, "Yes"
`readability-redundant-string-cstr `_, "Yes"
`readability-redundant-string-init `_, "Yes"
+   `readability-reference-to-constructed-temporary `_,
`readability-simplify-boolean-expr `_, "Yes"
`readability-simplify-subscript-expr `_, "Yes"
`readability-static-accessed-through-instance `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- cl

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked 2 inline comments as done.
PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18
+
+   const std::string& value("hello");
+

xgupta wrote:
> PiotrZSL wrote:
> > xgupta wrote:
> > > The below comment is not matching,  do you want to write - 
> > > `const std::string& str = std::string("hello");`
> > > ?
> > actually comment is +- fine, constructor to std::string will be called 
> > anyway, and for compiler I think those 2 are equal.
> > Will verify.
> but you have written below `reference variable ``str`` is assigned a 
> temporary object` and there is no str variable.
Ok, that's actually a bug. It's `value`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 544205.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Rename value to str in doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp
  
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-reference-to-constructed-temporary %t
+
+struct WithConstructor
+{
+WithConstructor(int, int);
+};
+
+struct WithoutConstructor
+{
+int a, b;
+};
+
+void test()
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: reference variable 'tmp1' extends the lifetime of a just-constructed temporary object 'const WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithConstructor& tmp1{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: reference variable 'tmp2' extends the lifetime of a just-constructed temporary object 'const WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithoutConstructor& tmp2{1,2};
+
+
+// CHECK-MESSAGES: :[[@LINE+1]]:22: warning: reference variable 'tmp3' extends the lifetime of a just-constructed temporary object 'WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithConstructor&& tmp3{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: reference variable 'tmp4' extends the lifetime of a just-constructed temporary object 'WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithoutConstructor&& tmp4{1,2};
+
+   WithConstructor tmp5{1,2};
+   WithoutConstructor tmp6{1,2};
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-reference-to-constructed-temporary
+
+readability-reference-to-constructed-temporary
+==
+
+Detects C++ code where a reference variable is used to extend the lifetime of
+a temporary object that has just been constructed.
+
+This construction is often the result of multiple code refactorings or a lack
+of developer knowledge, leading to confusion or subtle bugs. In most cases,
+what the developer really wanted to do is create a new variable rather than
+extending the lifetime of a temporary object.
+
+Examples of problematic code include:
+
+.. code-block:: c++
+
+   const std::string& str("hello");
+
+   struct Point { int x; int y; };
+   const Point& p = { 1, 2 };
+
+In the first example, a ``const std::string&`` reference variable ``str`` is
+assigned a temporary object created by the ``std::string("hello")``
+constructor. In the second example, a ``const Point&`` reference variable ``p``
+is assigned an object that is constructed from an initializer list ``{ 1, 2 }``.
+Both of these examples extend the lifetime of the temporary object to the
+lifetime of the reference variable, which can make it difficult to reason about
+and may lead to subtle bugs or misunderstanding.
+
+To avoid these issues, it is recommended to change the reference variable to a
+(``const``) value variable.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -379,6 +379,7 @@
`readability-redundant-smartptr-get `_, "Yes"
`readability-redundant-string-cstr `_, "Yes"
`readability-redundant-string-init `_, "Yes"
+   `readability-reference-to-constructed-temporary `_,
`readability-simplify-boolean-expr `_, "Yes"
`readability-simplify-subscript-expr `_, "Yes"
`readability-static-accessed-through-instance `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===

[PATCH] D156303: [clang-tidy] Remove AnalyzeTemporaryDestructors configuration option

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156303

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

[PATCH] D156438: [Docs] Fix code-blocks missing colon

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

Thank you.
LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156438

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

[PATCH] D149084: [clang-tidy] Added bugprone-multi-level-implicit-pointer-conversion check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG315946c57da2: [clang-tidy] Added 
bugprone-multi-level-implicit-pointer-conversion check (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D149084?vs=516480&id=544795#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149084

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  
clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp
  
clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s bugprone-multi-level-implicit-pointer-conversion %t
+
+using OneStar = void*;
+using OneStarFancy = OneStar;
+
+void takeFirstLevelVoidPtr(OneStar message);
+void takeFirstLevelConstVoidPtr(const OneStarFancy message);
+void takeFirstLevelConstVoidPtrConst(const void* const message);
+void takeSecondLevelVoidPtr(void** message);
+
+void** getSecondLevelVoidPtr();
+void* getFirstLevelVoidPtr();
+int** getSecondLevelIntPtr();
+int* getFirstLevelIntPtr();
+
+int table[5];
+
+void test()
+{
+  void** secondLevelVoidPtr;
+  int* firstLevelIntPtr;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  void* a = getSecondLevelVoidPtr();
+
+  void** b = getSecondLevelVoidPtr();
+  void* c = getFirstLevelVoidPtr();
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  void* d = getSecondLevelIntPtr();
+
+  takeFirstLevelVoidPtr(&table);
+
+  takeFirstLevelVoidPtr(firstLevelIntPtr);
+
+  takeFirstLevelVoidPtr(getFirstLevelIntPtr());
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelVoidPtr(secondLevelVoidPtr);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelConstVoidPtr(secondLevelVoidPtr);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void **' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelConstVoidPtrConst(secondLevelVoidPtr);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void ***' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelConstVoidPtrConst(&secondLevelVoidPtr);
+
+  takeSecondLevelVoidPtr(secondLevelVoidPtr);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelVoidPtr(getSecondLevelVoidPtr());
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelConstVoidPtr(getSecondLevelVoidPtr());
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void **' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelConstVoidPtrConst(getSecondLevelVoidPtr());
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]
+  takeFirstLevelVoidPtr(getSecondLevelIntPtr());
+
+  takeSecondLevelVoidPtr(getSecondLevelVoidPtr());
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -103,6 +103,7 @@
`bugprone-misplaced-pointer-arithmetic-in-alloc `_, "Yes"
`bugprone-misplaced-widening-cast `_,
`bugprone-move-forwarding-reference `_, "Yes"
+   `bugprone-mul

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp.
Herald added subscribers: mgrang, xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Sort printed options in --dump-config output.

Fixes: #64153


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156452

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,5 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s 
-check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep 
readability-identifier-naming | sort -c
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was 
deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,20 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions.end());
+
 IO.beginMapping();
 // Only output as a map
-for (auto &Key : Options) {
-  bool UseDefault;
-  void *SaveInfo;
-  IO.preflightKey(Key.getKey().data(), true, false, UseDefault, SaveInfo);
-  StringRef S = Key.getValue().Value;
-  IO.scalarString(S, needsQuotes(S));
+for (auto &Option : SortedOptions) {
+  bool UseDefault = false;
+  void *SaveInfo = nullptr;
+  IO.preflightKey(Option.first.data(), true, false, UseDefault, SaveInfo);
+  IO.scalarString(Option.second, needsQuotes(Option.second));
   IO.postflightKey(SaveInfo);
 }
 IO.endMapping();


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,5 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep readability-identifier-naming | sort -c
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,20 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions.end());
+
 IO.beginMapping();
 // Only output as a map
-for (auto

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked 3 inline comments as done.
PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:90
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {

carlosgalvezp wrote:
> I believe you can pass this directly to the constructor in the previous line, 
> to skip the `reserve()` part.
No, constructor takes `resize` argument, not `reserve`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

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

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 544852.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s 
-check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep 
readability-identifier-naming | sort --check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was 
deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions.end());
+
 IO.beginMapping();
 // Only output as a map
-for (auto &Key : Options) {
-  bool UseDefault;
-  void *SaveInfo;
-  IO.preflightKey(Key.getKey().data(), true, false, UseDefault, SaveInfo);
-  StringRef S = Key.getValue().Value;
-  IO.scalarString(S, needsQuotes(S));
+for (auto &Option : SortedOptions) {
+  bool UseDefault = false;
+  void *SaveInfo = nullptr;
+  IO.preflightKey(Option.first.data(), true, false, UseDefault, SaveInfo);
+  IO.scalarString(Option.second, needsQuotes(Option.second));
   IO.postflightKey(SaveInfo);
 }
 IO.endMapping();


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep readability-identifier-naming | sort --check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2cdb84376752: [clang-tidy] Sort options in --dump-config 
(authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s 
-check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep 
readability-identifier-naming | sort --check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was 
deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions.end());
+
 IO.beginMapping();
 // Only output as a map
-for (auto &Key : Options) {
-  bool UseDefault;
-  void *SaveInfo;
-  IO.preflightKey(Key.getKey().data(), true, false, UseDefault, SaveInfo);
-  StringRef S = Key.getValue().Value;
-  IO.scalarString(S, needsQuotes(S));
+for (auto &Option : SortedOptions) {
+  bool UseDefault = false;
+  void *SaveInfo = nullptr;
+  IO.preflightKey(Option.first.data(), true, false, UseDefault, SaveInfo);
+  IO.scalarString(Option.second, needsQuotes(Option.second));
   IO.postflightKey(SaveInfo);
 }
 IO.endMapping();


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-*" --dump-config | grep readability-identifier-naming | sort --check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_bac

[PATCH] D129706: [clang-tidy] Reimplement GlobList without relying on Regex.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

A litte overkill, there is llvm/include/llvm/Support/GlobPattern.h that could 
be used to simplify this code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129706

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

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

@thakis actually this test was already broken, simply folder '2' does not 
exist.. Let me fix that.
Thank you for information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

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

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3d9a64f7d4d6: [clang-tidy] Sort options in --dump-config 
(authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D156452?vs=544857&id=544903#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156452

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s 
-check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-naming" --dump-config 
%S/Inputs/config-files/- -- | grep "readability-identifier-naming\." | sort 
--check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was 
deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+SortedOptions.reserve(Options.size());
+for (auto &Key : Options) {
+  SortedOptions.emplace_back(Key.getKey(), Key.getValue().Value);
+}
+std::sort(SortedOptions.begin(), SortedOptions.end());
+
 IO.beginMapping();
 // Only output as a map
-for (auto &Key : Options) {
-  bool UseDefault;
-  void *SaveInfo;
-  IO.preflightKey(Key.getKey().data(), true, false, UseDefault, SaveInfo);
-  StringRef S = Key.getValue().Value;
-  IO.scalarString(S, needsQuotes(S));
+for (auto &Option : SortedOptions) {
+  bool UseDefault = false;
+  void *SaveInfo = nullptr;
+  IO.preflightKey(Option.first.data(), true, false, UseDefault, SaveInfo);
+  IO.scalarString(Option.second, needsQuotes(Option.second));
   IO.postflightKey(SaveInfo);
 }
 IO.endMapping();


Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -54,3 +54,6 @@
 // RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD6
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
+
+// Validate that check options are printed in alphabetical order:
+// RUN: clang-tidy --checks="-*,readability-identifier-naming" --dump-config %S/Inputs/config-files/- -- | grep "readability-identifier-naming\." | sort --check
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,8 @@
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was deprecated since
   :program:`clang-tidy` 16.
 
+- Improved `--dump-config` to print check options in alphabetical order.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 #include 
 
@@ -85,14 +86,21 @@
 void yamlize(IO &IO, ClangTidyOptions::OptionMap &Options, bool,
  EmptyContext &Ctx) {
   if (IO.outputting()) {
+// Ensure check options are sorted
+std::vector> SortedOptions;
+

[PATCH] D157367: [clang-tidy] Ignore delegate constructors in cppcoreguidelines-pro-type-member-init

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG894140b4fdde: [clang-tidy] Ignore delegate constructors in 
cppcoreguidelines-pro-type-member… (authored by PiotrZSL).

Changed prior to commit:
  https://reviews.llvm.org/D157367?vs=548085&id=549148#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157367

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -372,8 +372,7 @@
 class PositiveSelfInitialization : NegativeAggregateType
 {
   PositiveSelfInitialization() : PositiveSelfInitialization() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these bases: NegativeAggregateType
-  // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), 
PositiveSelfInitialization() {}
+  // This will be detected by -Wdelegating-ctor-cycles and there is no proper 
way to fix this
 };
 
 class PositiveIndirectMember {
@@ -579,3 +578,42 @@
 int C = 0;
   };
 };
+
+// Ignore issues from delegate constructors
+namespace PR37250 {
+  template 
+  struct A {
+A() : A(42) {}
+explicit A(int value) : value_(value) {}
+int value_;
+  };
+
+  struct B {
+B() : B(42) {}
+explicit B(int value) : value_(value) {}
+int value_;
+  };
+
+  template 
+  struct C {
+C() : C(T()) {}
+explicit C(T value) : value_(value) {}
+T value_;
+  };
+
+  struct V {
+unsigned size() const;
+  };
+
+  struct S {
+unsigned size_;
+
+S(unsigned size) : size_{size} {}
+
+template
+S(const U& u) : S(u.size()) {}
+  };
+
+  const V v;
+  const S s{v};
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@
   ` check to
   ignore delegate constructors.
 
+- Improved :doc:`cppcoreguidelines-pro-type-member-init
+  ` check to ignore
+  dependent delegate constructors.
+
 - Improved :doc:`cppcoreguidelines-pro-type-vararg
   ` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``, ``sizeof``, ...).
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -281,8 +281,14 @@
 
 void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) {
   auto IsUserProvidedNonDelegatingConstructor =
-  allOf(isUserProvided(),
-unless(anyOf(isInstantiated(), isDelegatingConstructor(;
+  allOf(isUserProvided(), unless(isInstantiated()),
+unless(isDelegatingConstructor()),
+ofClass(cxxRecordDecl().bind("parent")),
+unless(hasAnyConstructorInitializer(cxxCtorInitializer(
+isWritten(), unless(isMemberInitializer()),
+hasTypeLoc(loc(
+qualType(hasDeclaration(equalsBoundNode("parent");
+
   auto IsNonTrivialDefaultConstructor = allOf(
   isDefaultConstructor(), unless(isUserProvided()),
   hasParent(cxxRecordDecl(unless(isTriviallyDefaultConstructible();


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -372,8 +372,7 @@
 class PositiveSelfInitialization : NegativeAggregateType
 {
   PositiveSelfInitialization() : PositiveSelfInitialization() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these bases: NegativeAggregateType
-  // CHECK-FIXES: PositiveSelfInitialization() : NegativeAggregateType(), PositiveSelfInitialization() {}
+  // This will be detected by -Wdelegating-ctor-cycles and there is no proper way to fix this
 };
 
 class PositiveIndirectMember {
@@ -579,3 +578,42 @@
 int C = 0;
   };
 };
+
+// Ignore issues from delegate constructors
+namespace PR37250 {
+  template 
+  struct A {
+A() : A(42) {}
+explicit A(int value) : value_(value) {}
+int value_;
+  };
+
+  struct B {
+B() : B(42) {}
+explicit B(int value) : val

[PATCH] D157376: [clang-tidy] Ignore unevaluated context in cppcoreguidelines-pro-type-vararg

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f29f14d0257: [clang-tidy] Ignore unevaluated context in 
cppcoreguidelines-pro-type-vararg (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157376

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
@@ -66,3 +66,16 @@
   char *tmp1 = in;
   void *tmp2 = in;
 }
+
+namespace PR30542 {
+  struct X;
+  template 
+  char IsNullConstant(X*);
+  template 
+  char (&IsNullConstant(...))[2];
+
+  static_assert(sizeof(IsNullConstant(0)) == 1, "");
+  static_assert(sizeof(IsNullConstant(17)) == 2, "");
+
+  using Type = decltype(IsNullConstant(17));
+}
Index: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
@@ -7,7 +7,8 @@
 ``va_arg``.
 
 To allow for SFINAE use of vararg functions, a call is not flagged if a literal
-0 is passed as the only vararg argument.
+0 is passed as the only vararg argument or function is used in unevaluated
+context.
 
 Passing to varargs assumes the correct type will be read. This is fragile
 because it cannot generally be enforced to be safe in the language and so 
relies
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@
   ` check to
   ignore delegate constructors.
 
+- Improved :doc:`cppcoreguidelines-pro-type-vararg
+  ` check to ignore
+  false-positives in unevaluated context (e.g., ``decltype``, ``sizeof``, ...).
+
 - Improved :doc:`llvm-namespace-comment
   ` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "ProTypeVarargCheck.h"
+#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
@@ -133,7 +134,9 @@
 
   Finder->addMatcher(
   callExpr(callee(functionDecl(isVariadic(),
-   unless(hasAnyName(AllowedVariadics)
+   unless(hasAnyName(AllowedVariadics,
+   unless(hasAncestor(expr(matchers::hasUnevaluatedContext(,
+   unless(hasAncestor(typeLoc(
   .bind("callvararg"),
   this);
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-vararg.cpp
@@ -66,3 +66,16 @@
   char *tmp1 = in;
   void *tmp2 = in;
 }
+
+namespace PR30542 {
+  struct X;
+  template 
+  char IsNullConstant(X*);
+  template 
+  char (&IsNullConstant(...))[2];
+
+  static_assert(sizeof(IsNullConstant(0)) == 1, "");
+  static_assert(sizeof(IsNullConstant(17)) == 2, "");
+
+  using Type = decltype(IsNullConstant(17));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-vararg.rst
@@ -7,7 +7,8 @@
 ``va_arg``.
 
 To allow for SFINAE use of vararg functions, a call is not flagged if a literal
-0 is passed as the only vararg argument.
+0 is passed as the only vararg argument or function is used in unevaluated
+context.
 
 Passing to varargs assumes the correct type will be read. This is fragile
 because it cannot generally be enforced to be safe in the languag

[PATCH] D157649: [clang-tidy] Fix crash when diagnostic is emit with invalid location

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, aaron.ballman, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix crash when diagnostic is emit with invalid location,
but with attached valid ranges. Diagnostic can contain
invalid location, but SourceManager attached to it still
can be valid, use it in such case or fallback to known
SourceManager.

Fixes: #64602


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157649

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 
-DPR64602 | FileCheck -check-prefix=CHECK8 
-implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of 
type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested 
type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -436,9 +436,10 @@
 Errors.back());
 SmallString<100> Message;
 Info.FormatDiagnostic(Message);
-FullSourceLoc Loc;
-if (Info.getLocation().isValid() && Info.hasSourceManager())
-  Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+FullSourceLoc Loc(Info.getLocation(),
+  Info.hasSourceManager()
+  ? Info.getSourceManager()
+  : Context.DiagEngine->getSourceManager());
 Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
  Info.getFixItHints());
   }


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck -check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck --check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 -DPR64602 | FileCheck -check-prefix=CHECK8 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' [clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list ma

[PATCH] D157649: [clang-tidy] Fix crash when diagnostic is emit with invalid location

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 549263.
PiotrZSL added a comment.

Add call to DiagnosticEngine::hasSourceManager()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157649

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' 
%T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck 
-check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- 
-DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 
-implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy 
-checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion'
 %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck 
--check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 
-DPR64602 | FileCheck -check-prefix=CHECK8 
-implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' 
[clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of 
type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested 
type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -437,8 +437,11 @@
 SmallString<100> Message;
 Info.FormatDiagnostic(Message);
 FullSourceLoc Loc;
-if (Info.getLocation().isValid() && Info.hasSourceManager())
+if (Info.hasSourceManager())
   Loc = FullSourceLoc(Info.getLocation(), Info.getSourceManager());
+else if (Context.DiagEngine->hasSourceManager())
+  Loc = FullSourceLoc(Info.getLocation(),
+  Context.DiagEngine->getSourceManager());
 Converter.emitDiagnostic(Loc, DiagLevel, Message, Info.getRanges(),
  Info.getFixItHints());
   }


Index: clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -25,6 +25,7 @@
 // RUN: not clang-tidy -checks='-*,modernize-use-override' %T/diagnostics/input.cpp -- -DCOMPILATION_ERROR 2>&1 | FileCheck -check-prefix=CHECK6 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s
 // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined,clang-diagnostic-literal-conversion' %s -- -DMACRO_FROM_COMMAND_LINE -std=c++20 -Wno-macro-redefined | FileCheck --check-prefix=CHECK7 -implicit-check-not='{{warning:|error:}}' %s
+// RUN: not clang-tidy -checks='-*,modernize-use-override' %s -- -std=c++20 -DPR64602 | FileCheck -check-prefix=CHECK8 -implicit-check-not='{{warning:|error:}}' %s
 
 // CHECK1: error: no input files [clang-diagnostic-error]
 // CHECK1: error: no such file or directory: '{{.*}}nonexistent.cpp' [clang-diagnostic-error]
@@ -54,3 +55,18 @@
   // CHECK6: :[[@LINE-1]]:3: error: cannot take the address of an rvalue of type 'int' [clang-diagnostic-error]
 }
 #endif
+
+#ifdef PR64602 // Should not crash
+template 
+struct S
+{
+auto foo(auto);
+};
+
+template <>
+auto S<>::foo(auto)
+{
+return 1;
+}
+// CHECK8: error: template parameter list matching the non-templated nested type 'S<>' should be empty ('template<>') [clang-diagnostic-error]
+#endif
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -437,8 +437,11 @