alexander-shaposhnikov created this revision.
alexander-shaposhnikov added reviewers: gribozavr2, alexfh, rsmith.
alexander-shaposhnikov created this object with visibility "All Users".
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
alexander-shaposhnikov requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136224
Files:
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-cxx20.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -90,14 +90,16 @@
// 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() {}
Index:
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp
===================================================================
--- /dev/null
+++
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;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -158,7 +158,8 @@
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
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -217,6 +217,10 @@
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 differently
// when defaulted vs. empty.
@@ -224,6 +228,12 @@
anyOf(isUnion(),
has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()))))));
+ const LangOptions &LangOpts = getLangOpts();
+ auto IsPublicOrOutOfLineUntilCPP20 =
+ LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b
+ ? cxxConstructorDecl()
+ : cxxConstructorDecl(anyOf(isOutOfLine(), isPublic()));
+
// Destructor.
Finder->addMatcher(
cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
@@ -235,7 +245,8 @@
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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -90,14 +90,16 @@
// 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() {}
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-cxx20.cpp
===================================================================
--- /dev/null
+++ 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;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -158,7 +158,8 @@
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
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -217,6 +217,10 @@
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 differently
// when defaulted vs. empty.
@@ -224,6 +228,12 @@
anyOf(isUnion(),
has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()))))));
+ const LangOptions &LangOpts = getLangOpts();
+ auto IsPublicOrOutOfLineUntilCPP20 =
+ LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b
+ ? cxxConstructorDecl()
+ : cxxConstructorDecl(anyOf(isOutOfLine(), isPublic()));
+
// Destructor.
Finder->addMatcher(
cxxDestructorDecl(unless(hasParent(IsUnionLikeClass)), isDefinition())
@@ -235,7 +245,8 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
