llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/143550.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp (+12-3) - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h (+1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+8) - (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst (+5) - (added) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-macros.cpp (+57) - (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp (+26) ``````````diff diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp index 0de143dbb1b89..0b6b8d9c97135 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp @@ -18,6 +18,12 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +namespace { +AST_MATCHER(CXXRecordDecl, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} +} // namespace + SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AllowMissingMoveFunctions(Options.get( @@ -26,7 +32,8 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck( AllowMissingMoveFunctionsWhenCopyIsDeleted( Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", false)), AllowImplicitlyDeletedCopyOrMove( - Options.get("AllowImplicitlyDeletedCopyOrMove", false)) {} + Options.get("AllowImplicitlyDeletedCopyOrMove", false)), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} void SpecialMemberFunctionsCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -36,6 +43,7 @@ void SpecialMemberFunctionsCheck::storeOptions( AllowMissingMoveFunctionsWhenCopyIsDeleted); Options.store(Opts, "AllowImplicitlyDeletedCopyOrMove", AllowImplicitlyDeletedCopyOrMove); + Options.store(Opts, "IgnoreMacros", IgnoreMacros); } std::optional<TraversalKind> @@ -45,11 +53,12 @@ SpecialMemberFunctionsCheck::getCheckTraversalKind() const { } void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) { - auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted()); + const auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted()); + const ast_matchers::internal::Matcher<CXXRecordDecl> Anything = anything(); Finder->addMatcher( cxxRecordDecl( - unless(isImplicit()), + unless(isImplicit()), IgnoreMacros ? unless(isInMacro()) : Anything, eachOf(has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")), has(cxxConstructorDecl(isCopyConstructor(), IsNotImplicitOrDeleted) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h index dee01cb5a9fdd..c18ed7db055ba 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h @@ -69,6 +69,7 @@ class SpecialMemberFunctionsCheck : public ClangTidyCheck { const bool AllowMissingMoveFunctionsWhenCopyIsDeleted; const bool AllowImplicitlyDeletedCopyOrMove; ClassDefiningSpecialMembersMap ClassWithSpecialMembers; + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 19ccd1790e757..a79c10ed15a3d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -191,11 +191,19 @@ Changes in existing checks <clang-tidy/checks/concurrency/mt-unsafe>` check by fixing a false positive where ``strerror`` was flagged as MT-unsafe. +- Improved :doc:`cppcoreguidelines-special-member-functions + <clang-tidy/checks/cppcoreguidelines/special-member-functions>` check by + adding the option `IgnoreMacros` to ignore classes defined in macros. + - Improved :doc:`google-readability-namespace-comments <clang-tidy/checks/google/readability-namespace-comments>` check by adding the option `AllowOmittingNamespaceComments` to accept if a namespace comment is omitted entirely. +- Improved :doc:`hicpp-special-member-functions + <clang-tidy/checks/hicpp/special-member-functions>` check by adding the + option `IgnoreMacros` to ignore classes defined in macros. + - Improved :doc:`llvm-namespace-comment <clang-tidy/checks/llvm/namespace-comment>` check by adding the option `AllowOmittingNamespaceComments` to accept if a namespace comment is omitted diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst index 20f898fdab930..982d16fc8d23d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst @@ -85,3 +85,8 @@ Options struct A : boost::noncopyable { ~A() { std::cout << "dtor\n"; } }; + +.. option:: IgnoreMacros + + If set to `true`, the check will not give warnings for classes defined + inside macros. Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-macros.cpp new file mode 100644 index 0000000000000..58198979203ec --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions-macros.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: {cppcoreguidelines-special-member-functions.IgnoreMacros: false}}" -- + +class DefinesDestructor { + ~DefinesDestructor(); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesCopyConstructor { + DefinesCopyConstructor(const DefinesCopyConstructor &); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesNothing { +}; + +class DefinesEverything { + DefinesEverything(const DefinesEverything &); + DefinesEverything &operator=(const DefinesEverything &); + DefinesEverything(DefinesEverything &&); + DefinesEverything &operator=(DefinesEverything &&); + ~DefinesEverything(); +}; + +#define DEFINE_DESTRUCTOR_ONLY(ClassName) \ + class ClassName { \ + ~ClassName(); \ + }; + +#define DEFINE_COPY_CTOR_ONLY(ClassName) \ + class ClassName { \ + ClassName(const ClassName &); \ + }; + +#define DEFINE_CLASS_WITH_DTOR(ClassName) \ + class ClassName { \ + ~ClassName(); \ + }; + +DEFINE_DESTRUCTOR_ONLY(MacroDefinedClass1) +// CHECK-MESSAGES: [[@LINE-1]]:24: warning: class 'MacroDefinedClass1' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator +DEFINE_COPY_CTOR_ONLY(MacroDefinedClass2) +// CHECK-MESSAGES: [[@LINE-1]]:23: warning: class 'MacroDefinedClass2' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator +DEFINE_CLASS_WITH_DTOR(MacroDefinedClass3) +// CHECK-MESSAGES: [[@LINE-1]]:24: warning: class 'MacroDefinedClass3' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator + +// Test partial macro expansion +#define CLASS_NAME MacroNamedClass +class CLASS_NAME { + ~MacroNamedClass(); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'MacroNamedClass' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator + diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp index 60c945c8e20c3..28b515fb6afa3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/special-member-functions.cpp @@ -70,3 +70,29 @@ struct TemplateClass { // This should not cause problems. TemplateClass<int> InstantiationWithInt; TemplateClass<double> InstantiationWithDouble; + +#define DEFINE_DESTRUCTOR_ONLY(ClassName) \ + class ClassName { \ + ~ClassName(); \ + }; + +#define DEFINE_COPY_CTOR_ONLY(ClassName) \ + class ClassName { \ + ClassName(const ClassName &); \ + }; + +#define DEFINE_CLASS_WITH_DTOR(ClassName) \ + class ClassName { \ + ~ClassName(); \ + }; + +DEFINE_DESTRUCTOR_ONLY(MacroDefinedClass1) +DEFINE_COPY_CTOR_ONLY(MacroDefinedClass2) +DEFINE_CLASS_WITH_DTOR(MacroDefinedClass3) + +// Test partial macro expansion +#define CLASS_NAME MacroNamedClass +class CLASS_NAME { + ~MacroNamedClass(); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'MacroNamedClass' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator `````````` </details> https://github.com/llvm/llvm-project/pull/143550 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
