[PATCH] D86209: [clang-tidy] run-clang-tidy.py: Fix -allow-enabling-analyzer-alpha-checkers always being passed

[Joachim Priesner via Phabricator via cfe-commits]

jspam created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
jspam requested review of this revision.

The `action='store_true'` option of `argparse.add_argument` implicitly
generates a default value of False if the argument is not specified.
Thus, the `allow_enabling_alpha_checkers` argument of
`get_tidy_invocation` is never None.

Fixes: bbb7921da97ce03d2933e185a525c4e452b146b0 


[1] https://docs.python.org/3/library/argparse.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86209

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -84,7 +84,7 @@
 extra_arg, extra_arg_before, quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
-  if allow_enabling_alpha_checkers is not None:
+  if allow_enabling_alpha_checkers:
 start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -84,7 +84,7 @@
 extra_arg, extra_arg_before, quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
-  if allow_enabling_alpha_checkers is not None:
+  if allow_enabling_alpha_checkers:
 start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86209: [clang-tidy] run-clang-tidy.py: Fix -allow-enabling-analyzer-alpha-checkers always being passed

[Joachim Priesner via Phabricator via cfe-commits]

jspam added a comment.

Yes please. I am fine with the license agreement, and please use Joachim 
Priesner  as the commit author. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86209

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

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Joachim Priesner via Phabricator via cfe-commits]

jspam updated this revision to Diff 430375.
jspam added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
@@ -5,6 +5,8 @@
 
 void (int , int );
 
+int operator""_op(unsigned long long val);
+
 void f(int x, int y);
 void g() {
   // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does 
not match parameter name 'x'
@@ -14,7 +16,15 @@
   f(/*y=*/0, /*z=*/0);
   // CHECK-FIXES: {{^}}  f(/*y=*/0, /*z=*/0);
 
+  // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does 
not match parameter name 'x'
+  // CHECK-NOTES: [[@LINE-10]]:12: note: 'x' declared here
+  // CHECK-NOTES: [[@LINE+2]]:17: warning: argument name 'z' in comment does 
not match parameter name 'y'
+  // CHECK-NOTES: [[@LINE-12]]:19: note: 'y' declared here
+  f(/*y=*/0_op, /*z=*/0_op);
+  // CHECK-FIXES: {{^}}  f(/*y=*/0_op, /*z=*/0_op);
+
   f(/*x=*/1, /*y=*/1);
+  f(/*x=*/1_op, /*y=*/1_op);
 
   (0 /*=*/, /**/ 0); // Unsupported formats.
 }
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
@@ -5,6 +5,8 @@
 
 void (int , int );
 
+int operator""_op(unsigned long long val);
+
 void f(int x, int y);
 void g() {
   // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
@@ -14,7 +16,15 @@
   f(/*y=*/0, /*z=*/0);
   // CHECK-FIXES: {{^}}  f(/*y=*/0, /*z=*/0);
 
+  // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
+  // CHECK-NOTES: [[@LINE-10]]:12: note: 'x' declared here
+  // CHECK-NOTES: [[@LINE+2]]:17: warning: argument name 'z' in comment does not match parameter name 'y'
+  // CHECK-NOTES: [[@LINE-12]]:19: note: 'y' declared here
+  f(/*y=*/0_op, /*z=*/0_op);
+  // CHECK-FIXES: {{^}}  f(/*y=*/0_op, /*z=*/0_op);
+
   f(/*x=*/1, /*y=*/1);
+  f(/*x=*/1_op, /*y=*/1_op);
 
   (0 /*=*/, /**/ 0); // Unsupported formats.
 }
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125885: bugprone-argument-comment: Ignore calls to user-defined literals

[Joachim Priesner via Phabricator via cfe-commits]

jspam created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
jspam requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Without this change, code such as "f(/*param=*/1_op)" will check the
comment twice, once for the parameter of f (correct) and once for
the parameter of operator""_op (likely incorrect). The change removes
only the second check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125885

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
@@ -5,6 +5,8 @@
 
 void (int , int );
 
+int operator""_op(unsigned long long val);
+
 void f(int x, int y);
 void g() {
   // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does 
not match parameter name 'x'
@@ -14,7 +16,15 @@
   f(/*y=*/0, /*z=*/0);
   // CHECK-FIXES: {{^}}  f(/*y=*/0, /*z=*/0);
 
+  // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does 
not match parameter name 'x'
+  // CHECK-NOTES: [[@LINE-10]]:12: note: 'x' declared here
+  // CHECK-NOTES: [[@LINE+2]]:14: warning: argument name 'z' in comment does 
not match parameter name 'y'
+  // CHECK-NOTES: [[@LINE-12]]:19: note: 'y' declared here
+  f(/*y=*/0_op, /*z=*/0_op);
+  // CHECK-FIXES: {{^}}  f(/*y=*/0_op, /*z=*/0_op);
+
   f(/*x=*/1, /*y=*/1);
+  f(/*x=*/1_op, /*y=*/1_op);
 
   (0 /*=*/, /**/ 0); // Unsupported formats.
 }
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp
@@ -5,6 +5,8 @@
 
 void (int , int );
 
+int operator""_op(unsigned long long val);
+
 void f(int x, int y);
 void g() {
   // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
@@ -14,7 +16,15 @@
   f(/*y=*/0, /*z=*/0);
   // CHECK-FIXES: {{^}}  f(/*y=*/0, /*z=*/0);
 
+  // CHECK-NOTES: [[@LINE+4]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
+  // CHECK-NOTES: [[@LINE-10]]:12: note: 'x' declared here
+  // CHECK-NOTES: [[@LINE+2]]:14: warning: argument name 'z' in comment does not match parameter name 'y'
+  // CHECK-NOTES: [[@LINE-12]]:19: note: 'y' declared here
+  f(/*y=*/0_op, /*z=*/0_op);
+  // CHECK-FIXES: {{^}}  f(/*y=*/0_op, /*z=*/0_op);
+
   f(/*x=*/1, /*y=*/1);
+  f(/*x=*/1_op, /*y=*/1_op);
 
   (0 /*=*/, /**/ 0); // Unsupported formats.
 }
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Joachim Priesner via Phabricator via cfe-commits]

jspam updated this revision to Diff 430586.
jspam added a comment.

Update test as suggested by njames93.

bugprone-argument-comment-literals.cpp also contains some instances of a UDL 
used as an argument which didn't lead to problems before because the argument 
didn't have a name. So that case is covered as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
@@ -29,7 +29,7 @@
 void i(const char *c);
 void j(int a, int b, int c);
 
-double operator"" _km(long double);
+double operator"" _km(long double value);
 
 void test() {
   A a;
@@ -171,6 +171,8 @@
   g((1));
   // FIXME But we should not add argument comments here.
   g(_Generic(0, int : 0));
+
+  402.0_km;
 }
 
 void f(bool _with_underscores_);
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
@@ -29,7 +29,7 @@
 void i(const char *c);
 void j(int a, int b, int c);
 
-double operator"" _km(long double);
+double operator"" _km(long double value);
 
 void test() {
   A a;
@@ -171,6 +171,8 @@
   g((1));
   // FIXME But we should not add argument comments here.
   g(_Generic(0, int : 0));
+
+  402.0_km;
 }
 
 void f(bool _with_underscores_);
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -60,7 +60,7 @@
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  callExpr(unless(cxxOperatorCallExpr()),
+  callExpr(unless(cxxOperatorCallExpr()), unless(userDefinedLiteral()),
// NewCallback's arguments relate to the pointed function,
// don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
___
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

[Joachim Priesner via Phabricator via cfe-commits]

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

The following assertion fails when referencing an operator member:

  include/clang/AST/Decl.h:275: llvm::StringRef clang::NamedDecl::getName() 
const: Assertion `Name.isIdentifier() && "Name is not a simple identifier"' 
failed.


Repository:
  rG LLVM Github Monorepo

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) {}
 
@@ -360,6 +361,14 @@
 auto DDD = std::bind(::MemberFunction, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto DDD = [](auto && PH1) { PH1->MemberFunction(1); };
+
+auto EEE = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto EEE = [d] { d->operator()(1, 2); }
+
+auto FFF = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto FFF = [d] { d->operator bool(); }
   }
 };
 
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
@@ -690,7 +690,7 @@
   Stream << "->";
 }
 
-Stream << MethodDecl->getName();
+Stream << MethodDecl->getNameAsString();
   } else {
 Stream << " { return ";
 switch (LP.Callable.CE) {


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) {}
 
@@ -360,6 +361,14 @@
 auto DDD = std::bind(::MemberFunction, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto DDD = [](auto && PH1) { PH1->MemberFunction(1); };
+
+auto EEE = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto EEE = [d] { d->operator()(1, 2); }
+
+auto FFF = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto FFF = [d] { d->operator bool(); }
   }
 };
 
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
@@ -690,7 +690,7 @@
   Stream << "->";
 }
 
-Stream << MethodDecl->getName();
+Stream << MethodDecl->getNameAsString();
   } else {
 Stream << " { return ";
 switch (LP.Callable.CE) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128157: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Joachim Priesner via Phabricator via cfe-commits]

jspam updated this revision to Diff 439300.
jspam added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128157

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
@@ -272,6 +272,7 @@
 } // namespace Bugzilla_51912
 
 namespace macro_tests {
+#define MY_VIRTUAL virtual
 #define CONCAT(x, y) x##y
 
 // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is 
protected and virtual [cppcoreguidelines-virtual-class-destructor]
@@ -317,8 +318,17 @@
 protected:
   XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
 };
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is 
protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar6 {
+protected:
+  MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
+};
+
 #undef XMACRO
 #undef CONCAT
+#undef MY_VIRTUAL
 } // namespace macro_tests
 
 namespace FinalClassCannotBeBaseClass {
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -54,13 +54,17 @@
 return None;
 
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
-  SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
-  Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+  SourceLocation VirtualBeginSpellingLoc =
+  SM.getSpellingLoc(Destructor.getBeginLoc());
+  SourceLocation VirtualEndLoc = VirtualBeginSpellingLoc.getLocWithOffset(
+  Lexer::MeasureTokenLength(VirtualBeginSpellingLoc, SM, LangOpts));
 
   /// Range ends with \c StartOfNextToken so that any whitespace after \c
   /// virtual is included.
-  SourceLocation StartOfNextToken =
-  Lexer::findNextToken(VirtualEndLoc, SM, LangOpts)->getLocation();
+  Optional NextToken = Lexer::findNextToken(VirtualEndLoc, SM, 
LangOpts);
+  if (!NextToken)
+return None;
+  SourceLocation StartOfNextToken = NextToken->getLocation();
 
   return CharSourceRange::getCharRange(VirtualBeginLoc, StartOfNextToken);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp
@@ -272,6 +272,7 @@
 } // namespace Bugzilla_51912
 
 namespace macro_tests {
+#define MY_VIRTUAL virtual
 #define CONCAT(x, y) x##y
 
 // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
@@ -317,8 +318,17 @@
 protected:
   XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
 };
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar6 {
+protected:
+  MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
+};
+
 #undef XMACRO
 #undef CONCAT
+#undef MY_VIRTUAL
 } // namespace macro_tests
 
 namespace FinalClassCannotBeBaseClass {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -54,13 +54,17 @@
 return None;
 
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
-  SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
-  Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+  SourceLocation VirtualBeginSpellingLoc =
+  SM.getSpellingLoc(Destructor.getBeginLoc());
+  SourceLocation VirtualEndLoc = VirtualBeginSpellingLoc.getLocWithOffset(
+  Lexer::MeasureTokenLength(VirtualBeginSpellingLoc, SM, LangOpts));
 
   /// Range ends with \c StartOfNextToken so that any whitespace after \c
   /// virtual is included.
-  SourceLocation StartOfNextToken =
-  

[PATCH] D128157: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Joachim Priesner via Phabricator via cfe-commits]

jspam added a comment.

@LegalizeAdulthood Not sure what exactly you mean by "fold your changes into 
`checkers/cppcoreguidelines/virtual-class-destructor.cpp`". The only rebase 
conflict was in `VirtualClassDestructorCheck.cpp`. Please let me know if there 
is still something to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128157

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

[PATCH] D128157: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Joachim Priesner via Phabricator via cfe-commits]

jspam added a comment.

I see :) Yes, looks like Git's rename detection did its job.

Thanks for the review. Could you or someone else please merge this for me? 
Author: Joachim Priesner 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128157

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

[PATCH] D128157: [clang-tidy] cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Joachim Priesner via Phabricator via cfe-commits]

jspam closed this revision.
jspam added a comment.

No idea, I'll just close this then :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128157

___
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

[Joachim Priesner via Phabricator via cfe-commits]

jspam updated this revision to Diff 438265.
jspam added a comment.

Improve fixes

As suggested by njames93, use call syntax instead of operator().


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) {}
 
@@ -340,6 +341,7 @@
 
 struct E {
   void MemberFunction(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -360,6 +362,26 @@
 auto DDD = std::bind(::MemberFunction, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto DDD = [](auto && PH1) { PH1->MemberFunction(1); };
+
+auto EEE = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto EEE = [d] { (*d)(1, 2); }
+
+auto FFF = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto FFF = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto GGG = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto GGG = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto HHH = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto HHH = [d] { return d->operator bool(); }
+
+auto III = std::bind(::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto III = [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
@@ -685,12 +685,18 @@
 const BindArgument  = FunctionCallArgs.front();
 
 Stream << " { ";
-if (!isa(ignoreTemporariesAndPointers(ObjPtr.E))) {
-  Stream << ObjPtr.UsageIdentifier;
-  Stream << "->";
+if (!MethodDecl->getReturnType()->isVoidType()) {
+  Stream << "return ";
+}
+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 {
 Stream << " { return ";
 switch (LP.Callable.CE) {


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) {}
 
@@ -340,6 +341,7 @@
 
 struct E {
   void MemberFunction(int x) {}
+  int operator()(int x, int y) const { return x + y; }
 
   void testMemberFunctions() {
 D *d;
@@ -360,6 +362,26 @@
 auto DDD = std::bind(::MemberFunction, _1, 1);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
 // CHECK-FIXES: auto DDD = [](auto && PH1) { PH1->MemberFunction(1); };
+
+auto EEE = std::bind(::operator(), d, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto EEE = [d] { (*d)(1, 2); }
+
+auto FFF = std::bind(::operator(), , 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto FFF = [ObjectPtr = ] { (*ObjectPtr)(1, 2); }
+
+auto GGG = std::bind(::operator(), _1, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto GGG = [](auto && PH1) { (*PH1)(1, 2); };
+
+auto HHH = std::bind(::operator bool, d);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// CHECK-FIXES: auto HHH = [d] { return d->operator bool(); }
+
+auto III = std::bind(::operator(), this, 1, 2);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: 

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Joachim Priesner via Phabricator via cfe-commits]

jspam added a comment.

Would someone be able to commit this for me, as I do not have the necessary 
rights? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

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

[PATCH] D128157: cppcoreguidelines-virtual-class-destructor: Fix crash when "virtual" keyword is expanded from a macro

[Joachim Priesner via Phabricator via cfe-commits]

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

Check llvm::Optional before dereferencing it.

Compute VirtualEndLoc differently to avoid an assertion failure
in clang::SourceManager::getFileIDLoaded:

  Assertion `0 && "Invalid SLocOffset or bad function choice"' failed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128157

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -272,6 +272,7 @@
 } // namespace Bugzilla_51912
 
 namespace macro_tests {
+#define MY_VIRTUAL virtual
 #define CONCAT(x, y) x##y
 
 // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is 
protected and virtual [cppcoreguidelines-virtual-class-destructor]
@@ -317,6 +318,15 @@
 protected:
   XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
 };
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is 
protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar6 {
+protected:
+  MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
+};
+
 #undef XMACRO
 #undef CONCAT
+#undef MY_VIRTUAL
 } // namespace macro_tests
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -53,15 +53,17 @@
 return None;
 
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
-  SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
-  Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+  SourceLocation VirtualBeginSpellingLoc =
+  SM.getSpellingLoc(Destructor.getBeginLoc());
+  SourceLocation VirtualEndLoc = VirtualBeginSpellingLoc.getLocWithOffset(
+  Lexer::MeasureTokenLength(VirtualBeginSpellingLoc, SM, LangOpts));
 
   /// Range ends with \c StartOfNextToken so that any whitespace after \c
   /// virtual is included.
-  SourceLocation StartOfNextToken =
-  Lexer::findNextToken(VirtualEndLoc, SM, LangOpts)
-  .getValue()
-  .getLocation();
+  Optional NextToken = Lexer::findNextToken(VirtualEndLoc, SM, 
LangOpts);
+  if (!NextToken)
+return None;
+  SourceLocation StartOfNextToken = NextToken->getLocation();
 
   return CharSourceRange::getCharRange(VirtualBeginLoc, StartOfNextToken);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -272,6 +272,7 @@
 } // namespace Bugzilla_51912
 
 namespace macro_tests {
+#define MY_VIRTUAL virtual
 #define CONCAT(x, y) x##y
 
 // CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
@@ -317,6 +318,15 @@
 protected:
   XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
 };
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class FooBar6 {
+protected:
+  MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
+};
+
 #undef XMACRO
 #undef CONCAT
+#undef MY_VIRTUAL
 } // namespace macro_tests
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -53,15 +53,17 @@
 return None;
 
   SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
-  SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
-  Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+  SourceLocation VirtualBeginSpellingLoc =
+  SM.getSpellingLoc(Destructor.getBeginLoc());
+  SourceLocation 

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Joachim Priesner via Phabricator via cfe-commits]

jspam added a comment.

Thanks. Please use Joachim Priesner 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

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