[clang-tools-extra] 90d42b1 - [clang-tidy] Skip private default ctors in modernize-use-equals-default

[Alexander Shaposhnikov via cfe-commits]

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() {}