[PATCH] D133244: [clang-tidy] Readability-container-data-pointer adds new option to ignore Containers
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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
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
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
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
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
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.
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.
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
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.
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
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
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
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.
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.
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.
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.
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.
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.
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
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
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
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
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
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
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.
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.
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.
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
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
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
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
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
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
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
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()) {}