[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-02 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h


Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))


Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-03 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 457828.
felix642 added a comment.

+ Added test case and updated ReleaseNotes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" 
-- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -155,3 +155,25 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for 
accessing the data pointer instead of taking the address of the 0-th element 
[readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return holder.v.data();{{$}}
 }
+
+template
+struct ArrayType {
+  using size_type = size_t;
+
+  arrayType();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+
+private:
+  T _value[N];
+};
+
+// Don't issue a warning because of the config above.
+int *s() {
+  arrayType s;
+  return &s[0];
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
   The check now skips unions since in this case a default constructor with 
empty body
   is not equivalent to the explicitly defaulted one.
 
+- Improved `readability-container-data-pointer 
` check.
+
+  The check now skips containers that are defined in the option 
IgnoredContainers.  The default value is ::std::array.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" -- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-03 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 457829.
felix642 added a comment.

Fixed compilation issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" 
-- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -155,3 +155,25 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for 
accessing the data pointer instead of taking the address of the 0-th element 
[readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return holder.v.data();{{$}}
 }
+
+template
+struct ArrayType {
+  using size_type = size_t;
+
+  ArrayType();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+
+private:
+  T _value[N];
+};
+
+// Don't issue a warning because of the config above.
+int *s() {
+  ArrayType s;
+  return &s[0];
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
   The check now skips unions since in this case a default constructor with 
empty body
   is not equivalent to the explicitly defaulted one.
 
+- Improved `readability-container-data-pointer 
` check.
+
+  The check now skips containers that are defined in the option 
IgnoredContainers.  The default value is ::std::array.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" -- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -155,3 +155

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-05 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp:1
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" 
-- -fno-delayed-template-parsing
 

Eugene.Zelenko wrote:
> I think test should be separated to handle situations with and without option.
Hi @Eugene.Zelenko, 

I'm not familiar with clang-tidy's testing environment. What do you mean 
precisely by "test should be separated"? Does it mean I should define this test 
in a different .cpp with the appropriate tests?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-05 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 458073.
felix642 added a comment.

Improved readability of release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" 
-- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -155,3 +155,25 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for 
accessing the data pointer instead of taking the address of the 0-th element 
[readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return holder.v.data();{{$}}
 }
+
+template
+struct ArrayType {
+  using size_type = size_t;
+
+  ArrayType();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+
+private:
+  T _value[N];
+};
+
+// Don't issue a warning because of the config above.
+int *s() {
+  ArrayType s;
+  return &s[0];
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
   The check now skips unions since in this case a default constructor with 
empty body
   is not equivalent to the explicitly defaulted one.
 
+- Improved `readability-container-data-pointer 
` check.
+
+  The check now skips containers that are defined in the option 
`IgnoredContainers`.  The default value is `::std::array`.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -37,6 +37,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -22,14 +23,18 @@
 constexpr llvm::StringLiteral AddrOfContainerExprName =
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name, Context),
+  IgnoredContainers(utils::options::parseStringList(
+  Options.get("IgnoredContainers", DefaultIgnoredContainers))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
   cxxRecordDecl(
+  unless(hasAnyName(IgnoredContainers)),
   isSameOrDerivedFrom(
   namedDecl(
   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::ArrayType'}]}" -- -fno-delayed-template-parsing
 
 typedef __SIZE_TYPE__ size_t;

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 458943.
felix642 added a comment.

Changed tests to check with and without config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=CLASSIC %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -fno-delayed-template-parsing
+
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -58,20 +60,24 @@
 void g(size_t s) {
   std::vector b(s);
   f(&((b)[(z)]));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 void h() {
   std::string s;
   f(&((s).operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template  b(s);
   f(&b[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 template void i(size_t);
 
 void j(std::vector * const v) {
   f(&(*v)[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSI

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp:1
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- 
-fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- 
-config="{CheckOptions: [{key: 
readability-container-data-pointer.IgnoredContainers, value: '::arrayType'}]}" 
-- -fno-delayed-template-parsing
 

Eugene.Zelenko wrote:
> felix642 wrote:
> > Eugene.Zelenko wrote:
> > > I think test should be separated to handle situations with and without 
> > > option.
> > Hi @Eugene.Zelenko, 
> > 
> > I'm not familiar with clang-tidy's testing environment. What do you mean 
> > precisely by "test should be separated"? Does it mean I should define this 
> > test in a different .cpp with the appropriate tests?  
> I meant that dedicated test for new check option should be created. But will 
> be good idea to expand original one, so difference in behavior could be 
> observed.
I have added a new check-clang-tidy with a config to ignore std::basic_string. 

That way we can make sure that detections are still happening on containers 
that we do not ignore and we can also test that we don't detect containers that 
are defined in the config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-10-20 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-10-21 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:26
 constexpr llvm::StringLiteral AddressOfName = "address-of";
+const auto DefaultIgnoredContainers = "::std::array";
 

fwolff wrote:
> This is, of course, debatable, but I think I would prefer not to ignore 
> `std::array` by default. Note the documentation (emphasis mine):
> 
> > This **also** ensures that in the case that the container is empty, the 
> > data pointer access does not perform an errant memory access.
> 
> i.e. while you are correct that the danger of accessing an empty container 
> does not apply to an array of non-zero size (though you aren't checking 
> anywhere that the size is not zero AFAICS), this is not the **only** purpose 
> of this check, which explains why this check is found in the "readability" 
> category (instead of, say, "bugprone"). Therefore, I think the check is 
> useful even for `std::array`, and not ignoring it by default would be the 
> conservative choice here because that's what the check is currently doing.
Hi @fwolff,

I have to agree with you on this one. At first, when I read the initial issue 
on GitHub I had in mind that the Container-Data-Pointer check was used to 
reduce the risk of reading invalid memory, but since it's in the readability 
category this is not his main purpose. 
In that case, an empty default value seems more appropriate and users who which 
to ignore certain classes should add them manually.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp:59
 #define z (0)
 
 void g(size_t s) {

fwolff wrote:
> Regardless of whether or not `std::array` should be ignored by default, I 
> think it wouldn't hurt to add a test case for it somewhere in this file, 
> given that it was the impetus for this change, no?
If you look at the top of the file we are currently running 2 test cases. One 
without any config file and one with a config where we ignore 
std::basic::string.

This means that the test h() validates that we generate a warning when not 
using the new parameter and afterward also validates that we successfully 
ignore it if the key is present in the config file.

Since std::array will no longer be the default value I do not think that adding 
a specific test case for that container is needed. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-10-21 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 469830.
felix642 added a comment.

Updated documentation and code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=CLASSIC %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -fno-delayed-template-parsing
+
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -58,20 +60,24 @@
 void g(size_t s) {
   std::vector b(s);
   f(&((b)[(z)]));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 void h() {
   std::string s;
   f(&((s).operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template  b(s);
   f(&b[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 template void i(size_t);
 
 void j(std::vector * const v) {
   f(&(*v)[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-10-21 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 469831.
felix642 added a comment.

Updated ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=CLASSIC %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -fno-delayed-template-parsing
+
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -58,20 +60,24 @@
 void g(size_t s) {
   std::vector b(s);
   f(&((b)[(z)]));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 void h() {
   std::string s;
   f(&((s).operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template  b(s);
   f(&b[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(b.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-WITH-CONFIG: {{^  }}f(b.data());{{$}}
 }
 
 template void i(size_t);
 
 void j(std::vector * const v) {
   f(&(*v)[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-15 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-09-24 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2022-10-02 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

___
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.

2023-09-18 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

ping.


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] D159436: [clang-tidy] Add support for optional parameters in config.

2023-10-02 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 557545.
felix642 marked 3 inline comments as done.
felix642 added a comment.

Updated documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
  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
@@ -55,5 +55,12 @@
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
 
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
 // 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/test/clang-tidy/checkers/readability/function-size.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -8,6 +8,16 @@
 // RUN: readability-function-size.VariableThreshold: 1 \
 // RUN: }}'
 
+
+// RUN: %check_clang_tidy -check-suffixes=OPTIONAL %s readability-function-size %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-function-size.StatementThreshold: "-1", \
+// RUN: readability-function-size.BranchThreshold: "5", \
+// RUN: readability-function-size.ParameterThreshold: "none", \
+// RUN: readability-function-size.NestingThreshold: "", \
+// RUN: readability-function-size.VariableThreshold: "" \
+// RUN: }}'
+
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
 void foo1() {
@@ -103,9 +113,11 @@
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 25 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-5]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-6]]:6: note: 6 branches (threshold 5)
   if (true) { // 2
  int j;
   } else if (true) { // 2
@@ -123,7 +135,7 @@
   } else if (true) { // 2
  int j;
   }
-  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
+  // CHECK-MESSAGES: :[[@LINE-24]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,7 +12,7 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
+   Flag functions exceeding this number of lines. The default is `none` (ignore
the number of lines).
 
 .. option:: StatementThreshold
@@ -24,22 +24,22 @@
 .. option:: BranchThreshold
 
Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   `none` (ignore the number of branches).
 
 .. option:: ParameterThreshold
 
Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   is `none` (ignore the number of parameters).
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro

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

2023-10-02 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 557546.
felix642 added a comment.

Wrong check name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
  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
@@ -55,5 +55,12 @@
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
 
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
 // 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/test/clang-tidy/checkers/readability/function-size.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -8,6 +8,16 @@
 // RUN: readability-function-size.VariableThreshold: 1 \
 // RUN: }}'
 
+
+// RUN: %check_clang_tidy -check-suffixes=OPTIONAL %s readability-function-size %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-function-size.StatementThreshold: "-1", \
+// RUN: readability-function-size.BranchThreshold: "5", \
+// RUN: readability-function-size.ParameterThreshold: "none", \
+// RUN: readability-function-size.NestingThreshold: "", \
+// RUN: readability-function-size.VariableThreshold: "" \
+// RUN: }}'
+
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
 void foo1() {
@@ -103,9 +113,11 @@
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 25 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-5]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-6]]:6: note: 6 branches (threshold 5)
   if (true) { // 2
  int j;
   } else if (true) { // 2
@@ -123,7 +135,7 @@
   } else if (true) { // 2
  int j;
   }
-  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
+  // CHECK-MESSAGES: :[[@LINE-24]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,7 +12,7 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
+   Flag functions exceeding this number of lines. The default is `none` (ignore
the number of lines).
 
 .. option:: StatementThreshold
@@ -24,22 +24,22 @@
 .. option:: BranchThreshold
 
Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   `none` (ignore the number of branches).
 
 .. option:: ParameterThreshold
 
Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   is `none` (ignore the number of parameters).
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. The default is `none` (ignore the n

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

2023-08-13 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Improved check lambda-function-name with option IgnoreMacros to ignore warnings 
in macro expansion.
Relates to #62857 (https://github.com/llvm/llvm-project/issues/62857)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157829

Files:
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy -check-suffixes=,NO-CONFIG %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t -- -config="{CheckOptions: [{key: bugprone-lambda-function-name.IgnoreMacros, value: true}]}" --
+
 
 void Foo(const char* a, const char* b, int c) {}
 
@@ -12,11 +14,11 @@
   [] { __FUNCTION__; }();
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNC_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNCTION_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { EMBED_IN_ANOTHER_MACRO1; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
 }
 
 #define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__)
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
@@ -25,3 +25,11 @@
 
   Called from FancyFunction
   Now called from FancyFunction
+
+Options
+---
+
+.. option::  IgnoreMacros
+
+  The value `true` specifies that attempting to get the name of a function from
+  within a macro should not be diagnosed. The default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-lambda-function-name
+  ` check by adding option
+  `IgnoreMacros` to ignore warnings in macros.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   ` check
   to ignore ``static`` variables declared within the scope of
Index: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -13,6 +13,8 @@
 
 namespace clang::tidy::bugprone {
 
+inline constexpr bool DefaultIgnoreMacros = false;
+
 /// Detect when __func__ or __FUNCTION_

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

2023-08-14 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 550156.
felix642 added a comment.

Code review

Move constructor to cpp file
Added store options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157829

Files:
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy -check-suffixes=,NO-CONFIG %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t -- -config="{CheckOptions: [{key: bugprone-lambda-function-name.IgnoreMacros, value: true}]}" --
+
 
 void Foo(const char* a, const char* b, int c) {}
 
@@ -12,11 +14,11 @@
   [] { __FUNCTION__; }();
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNC_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNCTION_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { EMBED_IN_ANOTHER_MACRO1; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
 }
 
 #define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__)
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
@@ -25,3 +25,11 @@
 
   Called from FancyFunction
   Now called from FancyFunction
+
+Options
+---
+
+.. option::  IgnoreMacros
+
+  The value `true` specifies that attempting to get the name of a function from
+  within a macro should not be diagnosed. The default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-lambda-function-name
+  ` check by adding option
+  `IgnoreMacros` to ignore warnings in macros.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   ` check
   to ignore ``static`` variables declared within the scope of
Index: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -31,8 +31,9 @@
   };
   using SourceRangeSet = std::set;
 
-  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCall

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

2023-08-14 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 550159.
felix642 added a comment.

Updated commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157829

Files:
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy -check-suffixes=,NO-CONFIG %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t -- -config="{CheckOptions: [{key: bugprone-lambda-function-name.IgnoreMacros, value: true}]}" --
+
 
 void Foo(const char* a, const char* b, int c) {}
 
@@ -12,11 +14,11 @@
   [] { __FUNCTION__; }();
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNC_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNCTION_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { EMBED_IN_ANOTHER_MACRO1; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
 }
 
 #define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__)
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
@@ -25,3 +25,11 @@
 
   Called from FancyFunction
   Now called from FancyFunction
+
+Options
+---
+
+.. option::  IgnoreMacros
+
+  The value `true` specifies that attempting to get the name of a function from
+  within a macro should not be diagnosed. The default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-lambda-function-name
+  ` check by adding option
+  `IgnoreMacros` to ignore warnings in macros.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   ` check
   to ignore ``static`` variables declared within the scope of
Index: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -31,8 +31,9 @@
   };
   using SourceRangeSet = std::set;
 
-  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCallbacks(const SourceManager &SM, Preproce

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

2023-08-15 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 550559.
felix642 added a comment.

Updated format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157829

Files:
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/lambda-function-name.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy -check-suffixes=,NO-CONFIG %s bugprone-lambda-function-name %t
+// RUN: %check_clang_tidy %s bugprone-lambda-function-name %t -- -config="{CheckOptions: [{key: bugprone-lambda-function-name.IgnoreMacros, value: true}]}" --
+
 
 void Foo(const char* a, const char* b, int c) {}
 
@@ -12,11 +14,11 @@
   [] { __FUNCTION__; }();
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNC_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { FUNCTION_MACRO; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__FUNCTION__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
   [] { EMBED_IN_ANOTHER_MACRO1; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
+  // CHECK-MESSAGES-NO-CONFIG: :[[@LINE-1]]:8: warning: inside a lambda, '__func__' expands to the name of the function call operator; consider capturing the name of the enclosing function explicitly [bugprone-lambda-function-name]
 }
 
 #define FUNC_MACRO_WITH_FILE_AND_LINE Foo(__func__, __FILE__, __LINE__)
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/lambda-function-name.rst
@@ -25,3 +25,11 @@
 
   Called from FancyFunction
   Now called from FancyFunction
+
+Options
+---
+
+.. option::  IgnoreMacros
+
+  The value `true` specifies that attempting to get the name of a function from
+  within a macro should not be diagnosed. The default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-lambda-function-name
+  ` check by adding option
+  `IgnoreMacros` to ignore warnings in macros.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   ` check
   to ignore ``static`` variables declared within the scope of
Index: clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -31,8 +31,9 @@
   };
   using SourceRangeSet = std::set;
 
-  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP

[PATCH] D157829: [clang-tidy] Added a new option to lambda-function-name to ignore warnings in macro expansion

2023-08-15 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

HI @PiotrZSL, I do not have the rights to commit to the LLVM repository. Could 
you please commit those changes for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157829

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


[PATCH] D158338: [clang-tidy] [bugprone-implicit-widening-of-multiplication-result] Improved check to ignore false positives with integer literals.

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The following code is safe and should not trigger the warning
constexpr std::size_t k1Mb = 1024 * 1024;

Fixes #64732


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158338

Files:
  
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
@@ -107,10 +107,28 @@
 long n14(int a, int b, int c) {
   return a + b * c;
 }
+
 long n15(int a, int b, int c) {
   return a * b + c;
 }
 
+unsigned long n16()
+{
+  return (1024u) * 1024;
+}
+
+long n17(int a) {
+  return a + 1024 * 1024;
+}
+
+long n18(int a)
+{
+  return (a * 1024);
+  // CHECK-NOTES-ALL: :[[@LINE-1]]:11: warning: performing an implicit 
widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES-ALL: :[[@LINE-2]]:11: note: make conversion explicit to 
silence this warning
+  // CHECK-NOTES-ALL: :[[@LINE-3]]:11: note: perform multiplication in a wider 
type
+}
+
 #ifdef __cplusplus
 template 
 T2 template_test(T1 a, T1 b) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
+  
`
+  check to ignore false-positives with integer literals.
+
 - Improved :doc:`bugprone-lambda-function-name
   ` check by adding option
   `IgnoreMacros` to ignore warnings in macros.
Index: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
+++ 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
@@ -34,6 +34,13 @@
   return BO->getLHS()->IgnoreParens();
 }
 
+static bool hasIntegerLiteralOperators(const Expr* E)
+{
+  const auto *BO = dyn_cast(E);
+  return isa(BO->getLHS()->IgnoreParenImpCasts()) &&
+  isa(BO->getRHS()->IgnoreParenImpCasts());
+}
+
 ImplicitWideningOfMultiplicationResultCheck::
 ImplicitWideningOfMultiplicationResultCheck(StringRef Name,
 ClangTidyContext *Context)
@@ -89,6 +96,10 @@
   if (!LHS)
 return;
 
+  // Widening multiplication on Integer Literals.
+  if(hasIntegerLiteralOperators(E))
+return;
+
   // Ok, looks like we should diagnose this.
   diag(E->getBeginLoc(), "performing an implicit widening conversion to type "
  "%0 of a multiplication performed in type %1")


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
@@ -107,10 +107,28 @@
 long n14(int a, int b, int c) {
   return a + b * c;
 }
+
 long n15(int a, int b, int c) {
   return a * b + c;
 }
 
+unsigned long n16()
+{
+  return (1024u) * 1024;
+}
+
+long n17(int a) {
+  return a + 1024 * 1024;
+}
+
+long n18(int a)
+{
+  return (a * 1024);
+  // CHECK-NOTES-ALL: :[[@LINE-1]]:11: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES-ALL: :[[@LINE-2]]:11: note: make conversion explicit to silence this warning
+  // CHECK-NOTES-ALL: :[[@LINE-3]]:11: note: perform multiplication in a wider type
+}
+
 #ifdef __cplusplus
 template 
 T2 template_test(T1 a, T1 b) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-implicit-widening-of-multiplication-result

[PATCH] D158338: [clang-tidy] [bugprone-implicit-widening-of-multiplication-result] Improved check to ignore false positives with integer literals.

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 551771.
felix642 added a comment.

Fixed format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158338

Files:
  
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
@@ -107,10 +107,28 @@
 long n14(int a, int b, int c) {
   return a + b * c;
 }
+
 long n15(int a, int b, int c) {
   return a * b + c;
 }
 
+unsigned long n16()
+{
+  return (1024u) * 1024;
+}
+
+long n17(int a) {
+  return a + 1024 * 1024;
+}
+
+long n18(int a)
+{
+  return (a * 1024);
+  // CHECK-NOTES-ALL: :[[@LINE-1]]:11: warning: performing an implicit 
widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES-ALL: :[[@LINE-2]]:11: note: make conversion explicit to 
silence this warning
+  // CHECK-NOTES-ALL: :[[@LINE-3]]:11: note: perform multiplication in a wider 
type
+}
+
 #ifdef __cplusplus
 template 
 T2 template_test(T1 a, T1 b) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
+  
`
+  check to ignore false-positives with integer literals.
+
 - Improved :doc:`bugprone-lambda-function-name
   ` check by adding option
   `IgnoreMacros` to ignore warnings in macros.
Index: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
+++ 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
@@ -34,6 +34,12 @@
   return BO->getLHS()->IgnoreParens();
 }
 
+static bool hasIntegerLiteralOperators(const Expr *E) {
+  const auto *BO = dyn_cast(E);
+  return isa(BO->getLHS()->IgnoreParenImpCasts()) &&
+ isa(BO->getRHS()->IgnoreParenImpCasts());
+}
+
 ImplicitWideningOfMultiplicationResultCheck::
 ImplicitWideningOfMultiplicationResultCheck(StringRef Name,
 ClangTidyContext *Context)
@@ -89,6 +95,10 @@
   if (!LHS)
 return;
 
+  // Widening multiplication on Integer Literals.
+  if (hasIntegerLiteralOperators(E))
+return;
+
   // Ok, looks like we should diagnose this.
   diag(E->getBeginLoc(), "performing an implicit widening conversion to type "
  "%0 of a multiplication performed in type %1")


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-int.cpp
@@ -107,10 +107,28 @@
 long n14(int a, int b, int c) {
   return a + b * c;
 }
+
 long n15(int a, int b, int c) {
   return a * b + c;
 }
 
+unsigned long n16()
+{
+  return (1024u) * 1024;
+}
+
+long n17(int a) {
+  return a + 1024 * 1024;
+}
+
+long n18(int a)
+{
+  return (a * 1024);
+  // CHECK-NOTES-ALL: :[[@LINE-1]]:11: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int'
+  // CHECK-NOTES-ALL: :[[@LINE-2]]:11: note: make conversion explicit to silence this warning
+  // CHECK-NOTES-ALL: :[[@LINE-3]]:11: note: perform multiplication in a wider type
+}
+
 #ifdef __cplusplus
 template 
 T2 template_test(T1 a, T1 b) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -173,6 +173,10 @@
   `, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
+  `
+  check to ignore false-positives with integer literals.
+
 - Improved :doc:`bugprone-lambda-function-name
   ` check by adding option
   `IgnoreMacros` to ignore warnings in macros.
Index: clang-tools-extra/clang-tidy/bugprone/ImplicitWiden

[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158346

Files:
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -22,6 +22,10 @@
 };
 }
 
+namespace string_literals{
+string operator""s(const char *, size_t);
+}
+
 }
 
 template 
@@ -770,3 +774,17 @@
 bool testIgnoredDummyType(const IgnoredDummyType& value) {
   return value == IgnoredDummyType();
 }
+
+bool testStringLiterals(const std::string& s)
+{
+  using namespace std::string_literals;
+  return s == ""s;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be 
used
+  // CHECK-FIXES: {{^  }}return s.empty()
+}
+
+bool testNotEmptyStringLiterals(const std::string& s)
+{
+  using namespace std::string_literals;
+  return s == "foo"s;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -166,9 +166,11 @@
   this);
 
   // Comparison to empty string or empty constructor.
-  const auto WrongComparend = anyOf(
-  stringLiteral(hasSize(0)), cxxConstructExpr(isDefaultConstruction()),
-  cxxUnresolvedConstructExpr(argumentCountIs(0)));
+  const auto WrongComparend =
+  anyOf(stringLiteral(hasSize(0)),
+userDefinedLiteral(hasDescendant(stringLiteral(hasSize(0,
+cxxConstructExpr(isDefaultConstruction()),
+cxxUnresolvedConstructExpr(argumentCountIs(0)));
   // Match the object being compared.
   const auto STLArg =
   anyOf(unaryOperator(


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -22,6 +22,10 @@
 };
 }
 
+namespace string_literals{
+string operator""s(const char *, size_t);
+}
+
 }
 
 template 
@@ -770,3 +774,17 @@
 bool testIgnoredDummyType(const IgnoredDummyType& value) {
   return value == IgnoredDummyType();
 }
+
+bool testStringLiterals(const std::string& s)
+{
+  using namespace std::string_literals;
+  return s == ""s;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}return s.empty()
+}
+
+bool testNotEmptyStringLiterals(const std::string& s)
+{
+  using namespace std::string_literals;
+  return s == "foo"s;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -166,9 +166,11 @@
   this);
 
   // Comparison to empty string or empty constructor.
-  const auto WrongComparend = anyOf(
-  stringLiteral(hasSize(0)), cxxConstructExpr(isDefaultConstruction()),
-  cxxUnresolvedConstructExpr(argumentCountIs(0)));
+  const auto WrongComparend =
+  anyOf(stringLiteral(hasSize(0)),
+userDefinedLiteral(hasDescendant(stringLiteral(hasSize(0,
+cxxConstructExpr(isDefaultConstruction()),
+cxxUnresolvedConstructExpr(argumentCountIs(0)));
   // Match the object being compared.
   const auto STLArg =
   anyOf(unaryOperator(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D158338: [clang-tidy] [bugprone-implicit-widening-of-multiplication-result] Improved check to ignore false positives with integer literals.

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Hi @PiotrZSL thank you for taking the time to look at this revision.

I agree with you we should not silence a warning if no other tool can diagnose 
the issue.  I'm guessing that -Winteger-overflow does no trigger any warning on 
unsigned "overflow" since behavior is well defined in the standard :

> 46. This implies that unsigned arithmetic does not overflow because a result 
> that cannot be represented by the resulting unsigned integer type is reduced 
> modulo the number that is one greater than the largest value that can be 
> represented by the resulting unsigned integer type.

But on the other hand I have to agree with DenisYaroshevskiy that it is tedious 
to add a cast to every multiplication of integer literals on a large codebase. 
Especially when we know that those values do not overflow.

Maybe we should add some basic calculations when the operation is composed of 
integer literals, like you previously mentioned, to check if the operation 
actually overflows and print this warning if it does? 
In that case we could also improve the fix-it and suggest to add LU or U 
instead of static_cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158338

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


[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 551785.
felix642 added a comment.

Fixed tests and addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158346

Files:
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -22,6 +22,10 @@
 };
 }
 
+namespace string_literals{
+string operator""s(const char *, size_t);
+}
+
 }
 
 template 
@@ -461,7 +465,7 @@
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :90:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :94:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
@@ -492,17 +496,17 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(templated_container);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(templated_container);
 }
 
@@ -575,74 +579,74 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   while (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty()){{$}}
 
   do {
   }
   while (templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty());
 
   for (; templated_container.size();)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}for (; !templated_container.empty();){{$}}
 
   if (true && templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true && !templated_container.empty()){{$}}
 
   if (true || templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (tru

[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 marked 2 inline comments as done.
felix642 added a comment.

Hi @PiotrZSL, thank you for the feedback.

I have addressed most of your comments, but I'm not sure to understand what you 
mean by "Commit/Change description should be updated". Would you be able to 
clarify that for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158346

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


[PATCH] D158346: [clang-tidy] [readability-container-size-empty] improved check to detect missing usage of .empty() on string_literals Fixes #64547

2023-08-19 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 551789.
felix642 added a comment.

Clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158346

Files:
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -22,6 +22,10 @@
 };
 }
 
+namespace string_literals{
+string operator""s(const char *, size_t);
+}
+
 }
 
 template 
@@ -461,7 +465,7 @@
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :90:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :94:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
@@ -492,17 +496,17 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(templated_container);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(templated_container);
 }
 
@@ -575,74 +579,74 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   while (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty()){{$}}
 
   do {
   }
   while (templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty());
 
   for (; templated_container.size();)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}for (; !templated_container.empty();){{$}}
 
   if (true && templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true && !templated_container.empty()){{$}}
 
   if (true || templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :37:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true || !templated_contain

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

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 556326.
felix642 added a comment.

Updated Differential to truly support optional parameters


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 'c'
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Added support for optional parameters. If a parameter that requires an integer 
+  literal in the config file is set to `none`, `null`, `false`, or is left empty,
+  it will use its default value
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
===
--- clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
@@ -41,12 +41,12 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  const unsigned LineThreshold;
-  const unsigned StatementThreshold;
-  const unsigned BranchThreshold;
-  const unsigned ParameterThreshold;
-  const unsigned NestingThreshold;
-  const unsigned VariableThreshold;
+  const std::optional LineThreshold;
+  const std::optional StatementThreshold;
+  const std::optional BranchThreshold;
+  const std::optional ParameterThreshold;
+  const std::optional NestingThreshold;
+  const std::optional VariableThreshold;
 };
 
 } // namespace clang::tidy::readability
Index: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
@@ -126,12 +126,12 @@
 
 FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  LineThreshold(Options.get("LineThreshold", -1U)),
-  StatementThreshold(Options.get("StatementThreshold", 800U)),
-  BranchThreshold(Options.get("BranchThreshold", -1U)),
-  ParameterThreshold(Options.get("ParameterThreshold", -1U)),
-  NestingThreshold(Options.get("NestingThreshold", -1U)),
-  VariableThreshold(Options.get("VariableThreshold", -1U)) {}
+  LineThreshold(Options.get("LineThreshold")),
+  StatementThreshold(Options.get("StatementThreshold", 800U)),
+  BranchThreshold(Options.get("BranchThreshold")),
+  ParameterThreshold(Options.get("ParameterThreshold")),
+  NestingThreshold(Options.get("NestingThreshold")),
+  VariableThreshold(Options.get("VariableThreshold")) {}
 
 void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "LineThreshold", LineThreshold);
@@ -155,7 +155,7 @@
   const auto *Func = Result.Nodes.getNodeAs("func");
 
   FunctionASTVisitor Visitor;
-  Visitor.Info.NestingThreshold = NestingThreshold;
+  Visitor.Info.NestingThreshold = NestingThreshold.value_or(-1);
   Visitor.TraverseDecl(const_cast(Func));
   auto &FI = Visitor.I

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

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 556328.
felix642 added a comment.

Updated documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 'c'
Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   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.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This parameter
+   is disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This parameter
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This parameter is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
+   This parameter is disabled by default.
Please note that function parameters and variables declared in lambdas,
GNU Statement Expressions, and nested class inline functions are not counted.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Added support for optional parameters. Parameters that previously used -1 to disable
+  their effect can now be set to `none`, `null`, `false`, or left empty to ge the same
+  behaviour.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
===
--- clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
+++ clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
@@ -17,21 +17,21 @@
 ///
 /// These options are supported:
 ///
-///   * `LineThreshold` - flag functions exceeding this number of lines. The
-/// default is `-1` (ignore the number of lines).
+///   * `LineThreshold` - flag functions exceeding this number of lines. This
+/// parameter is disabled by default.
 ///   * `StatementThreshold` - fl

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

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 556331.
felix642 added a comment.

Removed false option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 'c'
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
@@ -55,5 +55,12 @@
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
 
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
 // 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/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   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.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This parameter
+   is disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This parameter
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This parameter is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
+   This parameter is disabled by default.
Please note that function parameters and variables declared in lambdas,
GNU Statement Expressions, and nested class inline functions are not counted.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Added suppor

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

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 556333.
felix642 added a comment.

Moved to a new method, changed tests, changed documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
  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
@@ -55,5 +55,12 @@
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
 
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
 // 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/test/clang-tidy/checkers/readability/function-size.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -8,6 +8,16 @@
 // RUN: readability-function-size.VariableThreshold: 1 \
 // RUN: }}'
 
+
+// RUN: %check_clang_tidy -check-suffixes=OPTIONAL %s readability-function-size %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-function-size.StatementThreshold: "-1", \
+// RUN: readability-function-size.BranchThreshold: "5", \
+// RUN: readability-function-size.ParameterThreshold: "none", \
+// RUN: readability-function-size.NestingThreshold: "", \
+// RUN: readability-function-size.VariableThreshold: "" \
+// RUN: }}'
+
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
 void foo1() {
@@ -103,9 +113,11 @@
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 25 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-5]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-6]]:6: note: 6 branches (threshold 5)
   if (true) { // 2
  int j;
   } else if (true) { // 2
@@ -123,7 +135,7 @@
   } else if (true) { // 2
  int j;
   }
-  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
+  // CHECK-MESSAGES: :[[@LINE-24]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   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.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This parameter
+   is disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This parameter
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may 

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

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Hi @PiotrZSL and @carlosgalvezp, I have updated my PR based on your comments. 
Let me know what you think.

  What if:
  
  "MinimumLength" is a boolean.
  It's default value (if not specified) is True.
  And a user wants to set it as "False" here
  
  Wouldn't that cause problems?

I have removed the "false" option to support optional for boolean types.

  this functionality should work only for integers...
  Idea is to have:
  
  template 
  std::enable_if_t, std::optional>
  getLocalOrGlobal(StringRef LocalName, std::optional Default) const;
  
   template 
   std::enable_if_t, std::optional>
   get(StringRef LocalName, std::optional Default) const;
  
  without impacting other functions... so that instead of doing tricks with -1 
in code, we could simply create real optional options, and be able to 'store' 
them as optional, this mean that --dump-config should dump them as "none".

I have moved the declaration to a new method which works only on integral types 
where the default value is std::optional. Let me know what you think.


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] D159436: [clang-tidy] Add support for optional parameters in config.

2023-09-08 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 556334.
felix642 added a comment.

Forgot to update a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tools-extra/clang-tidy/readability/FunctionSizeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
  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
@@ -55,5 +55,12 @@
 // CHECK-CHILD6: Checks: {{.*-llvm-qualified-auto'? *$}}
 // CHECK-CHILD6-NOT: modernize-use-using.IgnoreMacros
 
+// RUN: clang-tidy -dump-config \
+// RUN: --config='{CheckOptions: {readability-function-size.LineThreshold: ""}, \
+// RUN: Checks: readability-function-size}' \
+// RUN: %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD7
+// CHECK-CHILD7: readability-function-size.LineThreshold: none
+
+
 // 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/test/clang-tidy/checkers/readability/function-size.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/function-size.cpp
@@ -8,6 +8,16 @@
 // RUN: readability-function-size.VariableThreshold: 1 \
 // RUN: }}'
 
+
+// RUN: %check_clang_tidy -check-suffixes=OPTIONAL %s readability-function-size %t -- \
+// RUN: -config='{CheckOptions: { \
+// RUN: readability-function-size.StatementThreshold: "-1", \
+// RUN: readability-function-size.BranchThreshold: "5", \
+// RUN: readability-function-size.ParameterThreshold: "none", \
+// RUN: readability-function-size.NestingThreshold: "", \
+// RUN: readability-function-size.VariableThreshold: "" \
+// RUN: }}'
+
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
 void foo1() {
@@ -103,9 +113,11 @@
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 25 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-5]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
+  // CHECK-MESSAGES-OPTIONAL: :[[@LINE-6]]:6: note: 6 branches (threshold 5)
   if (true) { // 2
  int j;
   } else if (true) { // 2
@@ -123,7 +135,7 @@
   } else if (true) { // 2
  int j;
   }
-  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
+  // CHECK-MESSAGES: :[[@LINE-24]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   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.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This parameter
+   is disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This parameter
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the exp

[PATCH] D158691: [clang-tidy] Container-size-empty fixed c++ version in tests to support string_literals operator

2023-08-23 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158691

Files:
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
readability-container-size-empty %t -- \
 // RUN: -config="{CheckOptions: 
{readability-container-size-empty.ExcludedComparisonTypes: 
'::std::array;::IgnoredDummyType'}}" \
 // RUN: -- -fno-delayed-template-parsing -isystem %clang_tidy_headers
 #include 
@@ -23,7 +23,7 @@
 }
 
 namespace string_literals{
-string operator""_s(const char *, size_t);
+string operator""s(const char *, size_t);
 }
 
 }
@@ -778,7 +778,7 @@
 bool testStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == ""_s;
+  return s == ""s;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be 
used
   // CHECK-FIXES: {{^  }}return s.empty()
 }
@@ -786,5 +786,5 @@
 bool testNotEmptyStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == "foo"_s;
+  return s == "foo"s;
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: %check_clang_tidy -std=c++14-or-later %s readability-container-size-empty %t -- \
 // RUN: -config="{CheckOptions: {readability-container-size-empty.ExcludedComparisonTypes: '::std::array;::IgnoredDummyType'}}" \
 // RUN: -- -fno-delayed-template-parsing -isystem %clang_tidy_headers
 #include 
@@ -23,7 +23,7 @@
 }
 
 namespace string_literals{
-string operator""_s(const char *, size_t);
+string operator""s(const char *, size_t);
 }
 
 }
@@ -778,7 +778,7 @@
 bool testStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == ""_s;
+  return s == ""s;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}return s.empty()
 }
@@ -786,5 +786,5 @@
 bool testNotEmptyStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == "foo"_s;
+  return s == "foo"s;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-08-28 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The documentation would say that that default value for most parameters is -1. 
But since the parameter used in clang-tidy is an unsigned the value would get 
implicitly converted to 4294967295.

If a user tried to use -1 to disable this check he would receive an error 
saying that
the parameter is invalid.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159045

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not 
counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-08-28 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 554122.
felix642 added a comment.

Fixed commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159045

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not 
counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-08-28 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 554124.
felix642 added a comment.

Linked issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159045

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not 
counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.


Index: clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/function-size.rst
@@ -12,8 +12,8 @@
 
 .. option:: LineThreshold
 
-   Flag functions exceeding this number of lines. The default is `-1` (ignore
-   the number of lines).
+   Flag functions exceeding this number of lines. This option is disabled by 
+   default.
 
 .. option:: StatementThreshold
 
@@ -23,23 +23,23 @@
 
 .. option:: BranchThreshold
 
-   Flag functions exceeding this number of control statements. The default is
-   `-1` (ignore the number of branches).
+   Flag functions exceeding this number of control statements. This option is 
+   disabled by default.
 
 .. option:: ParameterThreshold
 
-   Flag functions that exceed a specified number of parameters. The default
-   is `-1` (ignore the number of parameters).
+   Flag functions that exceed a specified number of parameters. This option 
+   is disabled by default.
 
 .. option:: NestingThreshold
 
 Flag compound statements which create next nesting level after
 `NestingThreshold`. This may differ significantly from the expected value
-for macro-heavy code. The default is `-1` (ignore the nesting level).
+for macro-heavy code. This option is disabled by default.
 
 .. option:: VariableThreshold
 
Flag functions exceeding this number of variables declared in the body.
-   The default is `-1` (ignore the number of variables).
Please note that function parameters and variables declared in lambdas,
-   GNU Statement Expressions, and nested class inline functions are not counted.
+   GNU Statement Expressions, and nested class inline functions are not 
+   counted. This option is disabled by default.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-08-31 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

We can already disable those options if we don't define them in the config. 
Adding the possibility to provide optional values seems redundant to me. Do you 
see any reason why we would absolutely need to add this option to the config if 
we want to disable it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159045

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


[PATCH] D159045: [clang-tidy] Improved documentation for readability-function-size

2023-08-31 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

In that case, I agree with you, it would be helpful to add this feature. I 
think supporting an empty value rather than a boolean is preferable. We should 
maybe do that in another Differential though. I can open an issue on github and 
I'll open another diff when I'm ready. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159045

___
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.

2023-09-04 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

If a parameter value is either 'none', 'null', 'false', '-1' or '', we will in 
that case use the default value


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of 
similar type ('int') are easily swapped by mistake 
[bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 
'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 
'c'
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -184,8 +184,8 @@
 /// integral type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// ``std::nullopt``.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+///  return ``std::nullopt``.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
@@ -193,6 +193,9 @@
 std::enable_if_t, std::optional>
 get(StringRef LocalName) const {
   if (std::optional Value = get(LocalName)) {
+if (Value == "" || Value == "none" || Value == "null" ||
+  Value == "false" || (std::is_unsigned_v && Value == "-1"))
+  return std::nullopt;
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -286,8 +289,8 @@
 /// enum type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// \p Default.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+/// return \p Default.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return \p Default.


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last paramete

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

2023-09-04 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 555788.
felix642 added a comment.

Added entry to release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of 
similar type ('int') are easily swapped by mistake 
[bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 
'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 
'c'
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,9 @@
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Added support for optional parameters. If a parameter in the config file is
+  set to `none`, `null`, `false`, or is left empty, it will use its default 
value
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -184,8 +184,8 @@
 /// integral type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// ``std::nullopt``.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+///  return ``std::nullopt``.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
@@ -193,6 +193,9 @@
 std::enable_if_t, std::optional>
 get(StringRef LocalName) const {
   if (std::optional Value = get(LocalName)) {
+if (Value == "" || Value == "none" || Value == "null" ||
+  Value == "false" || (std::is_unsigned_v && Value == "-1"))
+  return std::nullopt;
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -286,8 +289,8 @@
 /// enum type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// \p Default.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+/// return \p Default.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return \p Default.


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b,

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

2023-09-04 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 555789.
felix642 added a comment.

Reworded release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159436

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \
+// RUN:  }}' --
+
+void a(int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 2 adjacent parameters of 'a' of 
similar type ('int') are easily swapped by mistake 
[bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:12: note: the first parameter in the range is 
'b'
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: the last parameter in the range is 
'c'
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Added support for optional parameters. If a parameter that requires an 
integer 
+  literal in the config file is set to `none`, `null`, `false`, or is left 
empty,
+  it will use its default value
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -184,8 +184,8 @@
 /// integral type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// ``std::nullopt``.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+///  return ``std::nullopt``.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return ``std::nullopt``.
@@ -193,6 +193,9 @@
 std::enable_if_t, std::optional>
 get(StringRef LocalName) const {
   if (std::optional Value = get(LocalName)) {
+if (Value == "" || Value == "none" || Value == "null" ||
+  Value == "false" || (std::is_unsigned_v && Value == "-1"))
+  return std::nullopt;
 T Result{};
 if (!StringRef(*Value).getAsInteger(10, Result))
   return Result;
@@ -286,8 +289,8 @@
 /// enum type ``T``.
 ///
 /// Reads the option with the check-local name \p LocalName from the
-/// ``CheckOptions``. If the corresponding key is not present, return
-/// \p Default.
+/// ``CheckOptions``. If the corresponding key is not present or empty,
+/// return \p Default.
 ///
 /// If the corresponding key can't be parsed as a ``T``, emit a
 /// diagnostic and return \p Default.


Index: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "none", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "null", \
+// RUN:  }}' --
+
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: { \
+// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \

[PATCH] D152764: [clang-tidy] Reserved-identifier: Improved AllowedIdentifiers option to support regular expressions

2023-06-18 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 532495.
felix642 added a comment.

Resolved comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152764

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-reserved-identifier.Invert, value: true}, \
-// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: std;reference_wrapper;ref;cref;type;get}, \
+// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: "std;reference_wrapper;^c?ref;type;get"}, \
 // RUN:   ]}' -- \
 // RUN:   -I%S/Inputs/reserved-identifier \
 // RUN:   -isystem %S/Inputs/reserved-identifier/system
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
@@ -53,5 +53,5 @@
 
 .. option:: AllowedIdentifiers
 
-   Semicolon-separated list of names that the check ignores. Default is an
+   Semicolon-separated list of regular expressions that the check ignores. Default is an
empty list.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -243,6 +243,10 @@
   ` check by adding support for
   other floating point representations in float constant like ``0.5L``.
 
+- Improved the :doc:`bugprone-reserved-identifier 
+  ` check by enhancing the 
+  `AllowedIdentifiers` option to support regular expressions.
+
 - Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
   in :doc:`bugprone-suspicious-include
   ` check.
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
@@ -30,7 +30,8 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/reserved-identifier.html
 class ReservedIdentifierCheck final : public RenamerClangTidyCheck {
   const bool Invert;
-  const std::vector AllowedIdentifiers;
+  const std::vector AllowedIdentifiersRaw;
+  const llvm::SmallVector AllowedIdentifiers;
 
 public:
   ReservedIdentifierCheck(StringRef Name, ClangTidyContext *Context);
@@ -46,6 +47,7 @@
   const SourceManager &SM) const override;
   DiagInfo getDiagInfo(const NamingCheckId &ID,
const NamingCheckFailure &Failure) const override;
+  llvm::SmallVector parseAllowedIdentifiers() const;
 };
 
 } // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -39,18 +39,35 @@
   return 0;
 }
 
+llvm::SmallVector
+ReservedIdentifierCheck::parseAllowedIdentifiers() const {
+  llvm::SmallVector AllowedIdentifiers;
+  AllowedIdentifiers.reserve(AllowedIdentifiersRaw.size());
+
+  for (const auto &Identifier : AllowedIdentifiersRaw) {
+AllowedIdentifiers.emplace_back(Identifier.str());
+if (!AllowedIdentifiers.back().isValid()) {
+  configurationDiag("Invalid allowed identifier regex '%0'") << Identifier;
+  AllowedIdentifiers.pop_back();
+}
+  }
+
+  return AllowedIdentifiers;
+}
+
 ReservedIdentifierCheck::ReservedIdentifierCheck(StringRef Name,
  ClangTidyContext *Context)
 : RenamerClangTidyCheck(Name, Context),
   Invert(Options.get("Invert", false)),
-  AllowedIdentifiers(utils::options::parseStringList(
-  Options.get("AllowedIdentifiers", ""))) {}
+  AllowedIdentifiersRaw(utils::options::parseStringList(
+  Options.get("AllowedIdentifiers", ""))),
+  AllowedIdentifiers(parseAllowedIdentifiers()) {}
 
 void ReservedIdentifierCheck:

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2023-06-05 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2023-06-07 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 529485.
felix642 added a comment.

Improved documentation
Removed duplicated messages in tests.
Added support for regular expressions
Added method to store options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,CLASSIC %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -fno-delayed-template-parsing
+
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -65,13 +67,15 @@
 void h() {
   std::string s;
   f(&((s).operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template ` check.
+
+  The check now skips containers that are defined in the option `IgnoredContainers`.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -30,6 +30,7 @@
 return LO.CPlusPlus11;
   }
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
 
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -37,6 +38,9 @@
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace readability
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,8 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -23,13 +25,22 @@
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
 
+void ContainerDataPointerCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoredContainers",
+utils::options::serializeStringList(IgnoredContainers));
+}
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Na

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2023-06-11 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 530360.
felix642 added a comment.

Updated documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,CLASSIC %s readability-container-data-pointer %t -- -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+
 #include 
 
 typedef __SIZE_TYPE__ size_t;
@@ -50,13 +52,15 @@
 void h() {
   std::string s;
   f(&((s).operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template ` check with new
+  `IgnoredContainers` option to ignore some containers.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -28,6 +28,7 @@
 return LO.CPlusPlus11;
   }
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
 
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -35,6 +36,9 @@
   std::optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector IgnoredContainers;
 };
 } // namespace clang::tidy::readability
 
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -8,6 +8,8 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -21,13 +23,22 @@
 "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
 
+void ContainerDataPointerCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoredContainers",
+utils::options::serializeStringList(IgnoredContainers));
+}
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context) {}
+: ClangTidyCheck(Name

[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers

2023-06-11 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 added a comment.

Hi @PiotrZSL, 
I have made the requested changes. If everything looks good to you would you 
mind committing this patch for me as I don't have commit access to the 
repository. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133244

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


[PATCH] D152764: [clang-tidy] Reserved-identifier: Improved AllowedIdentifiers option to support regular expressions

2023-06-12 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152764

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-reserved-identifier.Invert, value: true}, \
-// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: std;reference_wrapper;ref;cref;type;get}, \
+// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: "std;reference_wrapper;^c?ref;type;get"}, \
 // RUN:   ]}' -- \
 // RUN:   -I%S/Inputs/reserved-identifier \
 // RUN:   -isystem %S/Inputs/reserved-identifier/system
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
@@ -53,5 +53,5 @@
 
 .. option:: AllowedIdentifiers
 
-   Semicolon-separated list of names that the check ignores. Default is an
+   Semicolon-separated list of regular expressions that the check ignores. Default is an
empty list.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -411,6 +411,10 @@
   support unscoped enumerations through instances and fixed usage of anonymous
   structs or classes.
 
+- Improved option `AllowedIdentifiers` from :doc:`bugprone-reserved-identifier
+  ` to support regular
+  expressions.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
@@ -31,6 +31,7 @@
 class ReservedIdentifierCheck final : public RenamerClangTidyCheck {
   const bool Invert;
   const std::vector AllowedIdentifiers;
+  llvm::SmallVector AllowedIdentifiersRegex;
 
 public:
   ReservedIdentifierCheck(StringRef Name, ClangTidyContext *Context);
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -44,7 +44,13 @@
 : RenamerClangTidyCheck(Name, Context),
   Invert(Options.get("Invert", false)),
   AllowedIdentifiers(utils::options::parseStringList(
-  Options.get("AllowedIdentifiers", ""))) {}
+  Options.get("AllowedIdentifiers", ""))) {
+  for (const auto &Identifier : AllowedIdentifiers) {
+if (!llvm::Regex(Identifier).isValid())
+  configurationDiag("Invalid allowed identifier regex '%0'") << Identifier;
+AllowedIdentifiersRegex.emplace_back(Identifier.str());
+  }
+}
 
 void ReservedIdentifierCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   RenamerClangTidyCheck::storeOptions(Opts);
@@ -108,11 +114,14 @@
 static std::optional
 getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace,
const LangOptions &LangOpts, bool Invert,
-   ArrayRef AllowedIdentifiers) {
+   ArrayRef AllowedIdentifiers) {
   assert(!Name.empty());
-  if (llvm::is_contained(AllowedIdentifiers, Name))
-return std::nullopt;
 
+  if (llvm::any_of(AllowedIdentifiers, [&](const llvm::Regex &Regex) {
+return Regex.match(Name);
+  })) {
+return std::nullopt;
+  }
   // TODO: Check for names identical to language keywords, and other names
   // specifically reserved by language standards, e.g. C++ 'zombie names' and C
   // future library directions
@@ -159,14 +168,14 @@
  "Decl must be an explicit identifier with a name.");
   return getFailureInfoImpl(Dec

[PATCH] D152764: [clang-tidy] Reserved-identifier: Improved AllowedIdentifiers option to support regular expressions

2023-06-14 Thread Félix-Antoine Constantin via Phabricator via cfe-commits
felix642 updated this revision to Diff 531590.
felix642 added a comment.

Renamed allowedIdentifiers to allowedIdentifiersRaw
Moved regex parsing to a new method
Moved entry in releaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152764

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier-invert.cpp
@@ -1,7 +1,7 @@
 // RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-reserved-identifier.Invert, value: true}, \
-// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: std;reference_wrapper;ref;cref;type;get}, \
+// RUN: {key: bugprone-reserved-identifier.AllowedIdentifiers, value: "std;reference_wrapper;^c?ref;type;get"}, \
 // RUN:   ]}' -- \
 // RUN:   -I%S/Inputs/reserved-identifier \
 // RUN:   -isystem %S/Inputs/reserved-identifier/system
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/reserved-identifier.rst
@@ -53,5 +53,5 @@
 
 .. option:: AllowedIdentifiers
 
-   Semicolon-separated list of names that the check ignores. Default is an
+   Semicolon-separated list of regular expressions that the check ignores. Default is an
empty list.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -243,6 +243,10 @@
   ` check by adding support for
   other floating point representations in float constant like ``0.5L``.
 
+- Improved option `AllowedIdentifiers` from :doc:`bugprone-reserved-identifier
+  ` to support regular
+  expressions.
+
 - Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
   in :doc:`bugprone-suspicious-include
   ` check.
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
@@ -30,7 +30,8 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/reserved-identifier.html
 class ReservedIdentifierCheck final : public RenamerClangTidyCheck {
   const bool Invert;
-  const std::vector AllowedIdentifiers;
+  const std::vector AllowedIdentifiersRaw;
+  const llvm::SmallVector AllowedIdentifiers;
 
 public:
   ReservedIdentifierCheck(StringRef Name, ClangTidyContext *Context);
@@ -46,6 +47,7 @@
   const SourceManager &SM) const override;
   DiagInfo getDiagInfo(const NamingCheckId &ID,
const NamingCheckFailure &Failure) const override;
+  llvm::SmallVector parseAllowedIdentifiers() const;
 };
 
 } // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -39,18 +39,33 @@
   return 0;
 }
 
+llvm::SmallVector
+ReservedIdentifierCheck::parseAllowedIdentifiers() const {
+  llvm::SmallVector AllowedIdentifiers;
+  AllowedIdentifiers.reserve(AllowedIdentifiersRaw.size());
+
+  for (const auto &Identifier : AllowedIdentifiersRaw) {
+AllowedIdentifiers.emplace_back(Identifier.str());
+if (!AllowedIdentifiers.back().isValid())
+  configurationDiag("Invalid allowed identifier regex '%0'") << Identifier;
+  }
+
+  return AllowedIdentifiers;
+}
+
 ReservedIdentifierCheck::ReservedIdentifierCheck(StringRef Name,
  ClangTidyContext *Context)
 : RenamerClangTidyCheck(Name, Context),
   Invert(Options.get("Invert", false)),
-  AllowedIdentifiers(utils::options::parseStringList(
-  Options.get("AllowedIdentifiers", ""))) {}
+  AllowedIdentifiersRaw(utils::options::parseStringList(
+  Options.get("AllowedIdentifiers", ""))),
+  AllowedIdentifiers(parseAllowedIdentifiers()) {}