Author: Alexander Shaposhnikov Date: 2022-10-20T22:04:33Z New Revision: 90d42b1cab04b76e9e934f1e5076f014d1195e72
URL: https://github.com/llvm/llvm-project/commit/90d42b1cab04b76e9e934f1e5076f014d1195e72 DIFF: https://github.com/llvm/llvm-project/commit/90d42b1cab04b76e9e934f1e5076f014d1195e72.diff LOG: [clang-tidy] Skip private default ctors in modernize-use-equals-default For c++17 (i.e. before c++20) making a private default ctor explicitly defaulted might expose the previously intentionally disallowed initializations, e.g. class Tag { Tag() {} friend class Widget; }; is not equivalent to class Tag { Tag() = default; friend class Widget; }; since in the latter case Tag is treated as an aggregate despite having a declaration of the default constructor. This diff makes modernize-use-equals-default skip in-class empty nonpublic default ctors to avoid code breakages. Test plan: ninja check-all Differential revision: https://reviews.llvm.org/D136224 Added: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx17.cpp clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp Modified: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp index 165840b72e7aa..e72ea76114783 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp @@ -217,6 +217,10 @@ void UseEqualsDefaultCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreMacros", IgnoreMacros); } +namespace { +AST_MATCHER(CXXMethodDecl, isOutOfLine) { return Node.isOutOfLine(); } +} // namespace + void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) { // Skip unions/union-like classes since their constructors behave diff erently // when defaulted vs. empty. @@ -224,6 +228,12 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) { anyOf(isUnion(), has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion())))))); + const LangOptions &LangOpts = getLangOpts(); + auto IsPublicOrOutOfLineUntilCPP20 = + LangOpts.CPlusPlus20 + ? cxxConstructorDecl() + : cxxConstructorDecl(anyOf(isOutOfLine(), isPublic())); + // Destructor. Finder->addMatcher( cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition()) @@ -235,7 +245,8 @@ void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) { anyOf( // Default constructor. allOf(unless(hasAnyConstructorInitializer(isWritten())), - unless(isVariadic()), parameterCountIs(0)), + unless(isVariadic()), parameterCountIs(0), + IsPublicOrOutOfLineUntilCPP20), // Copy constructor. allOf(isCopyConstructor(), // Discard constructors that can be used as a copy diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 735fedbce5496..68995a9d3f190 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -158,7 +158,8 @@ Changes in existing checks The check now skips unions/union-like classes since in this case a default constructor with empty body is not equivalent to the explicitly defaulted one, variadic constructors since they cannot be explicitly defaulted. The check also skips copy assignment operators - with nonstandard return types. The check is restricted to c++11-or-later. + with nonstandard return types, private/protected default constructors for C++17 or earlier. + The check is restricted to C++11 or later. - Change the default behavior of :doc:`readability-avoid-const-params-in-decls <clang-tidy/checks/readability/avoid-const-params-in-decls>` to not diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx17.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx17.cpp new file mode 100644 index 0000000000000..3bbc6296816b1 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx17.cpp @@ -0,0 +1,17 @@ +// RUN: %check_clang_tidy -std=c++17 %s modernize-use-equals-default %t -- -- -fno-delayed-template-parsing -fexceptions + +// Private constructor/destructor. +class Priv { + Priv() {} + ~Priv() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' + // CHECK-FIXES: ~Priv() = default; +}; + +class PrivOutOfLine { + PrivOutOfLine(); +}; + +PrivOutOfLine::PrivOutOfLine() {} +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use '= default' +// CHECK-FIXES: PrivOutOfLine::PrivOutOfLine() = default; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp new file mode 100644 index 0000000000000..38dfa5ebaea7e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy -std=c++20 %s modernize-use-equals-default %t -- -- -fno-delayed-template-parsing -fexceptions + +// Private constructor/destructor. +class Priv { + Priv() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' + // CHECK-FIXES: Priv() = default; + ~Priv() {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' + // CHECK-FIXES: ~Priv() = default; +}; + +class PrivOutOfLine { + PrivOutOfLine(); +}; + +PrivOutOfLine::PrivOutOfLine() {} +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use '= default' +// CHECK-FIXES: PrivOutOfLine::PrivOutOfLine() = default; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp index 493cfa21a8371..6482c3a82241e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp @@ -90,14 +90,16 @@ class CM { // Private constructor/destructor. class Priv { - Priv() {} - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' - // CHECK-FIXES: Priv() = default; + Priv(); ~Priv() {}; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' // CHECK-FIXES: ~Priv() = default; }; +Priv::Priv() {} +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use '= default' +// CHECK-FIXES: Priv::Priv() = default; + // struct. struct ST { ST() {} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
