llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Helmut Januschka (hjanuschka)
<details>
<summary>Changes</summary>
Add new clang-tidy check that finds classes which could be aggregates if their
trivial forwarding constructors were removed.
A constructor is considered a trivial forwarder when it takes one parameter per
non-static data member, initializes each member from the corresponding
parameter in declaration order, and has an empty body. Removing such
constructors enables aggregate initialization and, in C++20, designated
initializers.
```cpp
// Before
struct Point {
int X;
int Y;
Point(int X, int Y) : X(X), Y(Y) {}
};
Point p(1, 2);
// After -- remove the constructor
struct Point {
int X;
int Y;
};
Point p{1, 2}; // aggregate initialization
Point q{.X = 1, .Y = 2}; // designated initializers (C++20)
```
The check will **not** flag a class if it:
- has virtual functions
- has private or protected non-static data members
- has virtual, private, or protected base classes
- has base classes (before C++17)
- has a user-provided destructor
- has additional non-trivial constructors
- is a template specialization
---
Full diff: https://github.com/llvm/llvm-project/pull/182061.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
- (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
(+3)
- (added) clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp (+215)
- (added) clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.h (+39)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+7)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-aggregate.rst
(+43)
- (added)
clang-tools-extra/test/clang-tidy/checkers/modernize/use-aggregate.cpp (+145)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index cc4cc7a02b594..92aa7b9388cb2 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -31,6 +31,7 @@ add_clang_library(clangTidyModernizeModule STATIC
ShrinkToFitCheck.cpp
TypeTraitsCheck.cpp
UnaryStaticAssertCheck.cpp
+ UseAggregateCheck.cpp
UseAutoCheck.cpp
UseBoolLiteralsCheck.cpp
UseConstraintsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fcb860d8c5298..9c6758cf99ecd 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -34,6 +34,7 @@
#include "UseAutoCheck.h"
#include "UseBoolLiteralsCheck.h"
#include "UseConstraintsCheck.h"
+#include "UseAggregateCheck.h"
#include "UseDefaultMemberInitCheck.h"
#include "UseDesignatedInitializersCheck.h"
#include "UseEmplaceCheck.h"
@@ -116,6 +117,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<TypeTraitsCheck>("modernize-type-traits");
CheckFactories.registerCheck<UnaryStaticAssertCheck>(
"modernize-unary-static-assert");
+ CheckFactories.registerCheck<UseAggregateCheck>(
+ "modernize-use-aggregate");
CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
CheckFactories.registerCheck<UseBoolLiteralsCheck>(
"modernize-use-bool-literals");
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp
new file mode 100644
index 0000000000000..a1c128fe53483
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp
@@ -0,0 +1,215 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseAggregateCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+/// Check whether \p Ctor is a trivial forwarding constructor: each parameter
+/// is used to initialise the corresponding member (in declaration order) and
+/// the body is empty.
+static bool isTrivialForwardingConstructor(const CXXConstructorDecl *Ctor) {
+ if (!Ctor || !Ctor->hasBody())
+ return false;
+
+ // Body must be an empty compound statement.
+ const auto *Body = dyn_cast<CompoundStmt>(Ctor->getBody());
+ if (!Body || !Body->body_empty())
+ return false;
+
+ const CXXRecordDecl *Record = Ctor->getParent();
+
+ // Collect non-static data members in declaration order.
+ SmallVector<const FieldDecl *, 8> Fields;
+ for (const auto *Field : Record->fields())
+ Fields.push_back(Field);
+
+ // Number of parameters must match number of fields.
+ if (Ctor->getNumParams() != Fields.size())
+ return false;
+
+ // Number of member initializers must match number of fields (no base
+ // class inits, no extra inits).
+ unsigned NumMemberInits = 0;
+ for (const auto *Init : Ctor->inits()) {
+ if (Init->isMemberInitializer())
+ ++NumMemberInits;
+ else
+ return false; // base class or delegating init -- bail out
+ }
+ if (NumMemberInits != Fields.size())
+ return false;
+
+ // Walk initializers and check each one initializes the matching field
+ // from the matching parameter.
+ unsigned FieldIdx = 0;
+ for (const auto *Init : Ctor->inits()) {
+ if (!Init->isMemberInitializer())
+ return false;
+
+ // Must match the field at the current position.
+ if (Init->getMember() != Fields[FieldIdx])
+ return false;
+
+ const Expr *InitExpr = Init->getInit()->IgnoreImplicit();
+
+ // Handle CXXConstructExpr wrapping the parameter (for class types).
+ if (const auto *Construct = dyn_cast<CXXConstructExpr>(InitExpr)) {
+ if (Construct->getNumArgs() != 1)
+ return false;
+ // Must be a copy or move constructor call.
+ const CXXConstructorDecl *InitCtor = Construct->getConstructor();
+ if (!InitCtor->isCopyOrMoveConstructor())
+ return false;
+ InitExpr = Construct->getArg(0)->IgnoreImplicit();
+ }
+
+ // The init expression must be a DeclRefExpr to the corresponding param.
+ const auto *DRE = dyn_cast<DeclRefExpr>(InitExpr);
+ if (!DRE)
+ return false;
+ const auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl());
+ if (!PVD || PVD != Ctor->getParamDecl(FieldIdx))
+ return false;
+
+ ++FieldIdx;
+ }
+
+ return true;
+}
+
+/// Check whether the class would be a valid aggregate if all user-declared
+/// constructors were removed.
+static bool canBeAggregate(const CXXRecordDecl *Record,
+ const LangOptions &LangOpts) {
+ if (!Record || !Record->hasDefinition())
+ return false;
+
+ // Must not have virtual functions.
+ if (Record->isPolymorphic())
+ return false;
+
+ // Must not have private or protected non-static data members.
+ for (const auto *Field : Record->fields())
+ if (Field->getAccess() != AS_public)
+ return false;
+
+ // Must not have virtual, private, or protected base classes.
+ for (const auto &Base : Record->bases()) {
+ if (Base.isVirtual())
+ return false;
+ if (Base.getAccessSpecifier() != AS_public)
+ return false;
+ }
+
+ // C++17 and later allow non-virtual public base classes in aggregates.
+ // Before C++17, aggregates cannot have base classes at all.
+ if (!LangOpts.CPlusPlus17 && Record->getNumBases() > 0)
+ return false;
+
+ return true;
+}
+
+void UseAggregateCheck::registerMatchers(MatchFinder *Finder) {
+ // Match class/struct definitions that have at least one user-provided
+ // constructor.
+ Finder->addMatcher(
+ cxxRecordDecl(
+ isDefinition(),
+ unless(isImplicit()),
+ unless(isLambda()),
+ has(cxxConstructorDecl(isUserProvided(), unless(isDeleted()))))
+ .bind("record"),
+ this);
+}
+
+void UseAggregateCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Record = Result.Nodes.getNodeAs<CXXRecordDecl>("record");
+ if (!Record)
+ return;
+
+ // Skip records in system headers.
+ if (Record->getLocation().isInvalid() ||
+ Result.SourceManager->isInSystemHeader(Record->getLocation()))
+ return;
+
+ // Skip template specializations to avoid false positives.
+ if (isa<ClassTemplateSpecializationDecl>(Record))
+ return;
+
+ // Skip if no fields (empty structs are already aggregates by default).
+ if (Record->field_empty())
+ return;
+
+ // Check aggregate preconditions (ignoring constructors).
+ if (!canBeAggregate(Record, getLangOpts()))
+ return;
+
+ // Collect all user-declared constructors.
+ SmallVector<const CXXConstructorDecl *, 4> UserCtors;
+ for (const auto *Decl : Record->decls()) {
+ const auto *Ctor = dyn_cast<CXXConstructorDecl>(Decl);
+ if (!Ctor || Ctor->isImplicit())
+ continue;
+
+ // If there is any user-declared constructor that is not a trivial
+ // forwarding constructor and not defaulted/deleted, bail out. We cannot
+ // safely suggest removing it.
+ if (Ctor->isDeleted() || Ctor->isDefaulted()) {
+ // Explicit default/delete still counts as user-declared in C++20
+ // aggregate rules, but we focus on user-provided constructors.
+ // In C++20 mode, even =default prevents aggregate, so we should
+ // flag those too.
+ if (getLangOpts().CPlusPlus20)
+ UserCtors.push_back(Ctor);
+ continue;
+ }
+
+ if (!isTrivialForwardingConstructor(Ctor))
+ return; // Non-trivial constructor -- not safe to remove
+
+ UserCtors.push_back(Ctor);
+ }
+
+ if (UserCtors.empty())
+ return;
+
+ // Check that there is no user-provided destructor.
+ if (const auto *Dtor = Record->getDestructor())
+ if (Dtor->isUserProvided())
+ return;
+
+ // Find the primary forwarding constructor to diagnose on.
+ const CXXConstructorDecl *PrimaryCtor = nullptr;
+ for (const auto *Ctor : UserCtors) {
+ if (!Ctor->isDeleted() && !Ctor->isDefaulted()) {
+ PrimaryCtor = Ctor;
+ break;
+ }
+ }
+
+ if (!PrimaryCtor)
+ return;
+
+ // Emit diagnostic on the class, with a note on the constructor.
+ diag(Record->getLocation(),
+ "'%0' can be an aggregate type if the forwarding constructor "
+ "is removed")
+ << Record->getName();
+ diag(PrimaryCtor->getLocation(),
+ "remove this constructor to enable aggregate initialization",
+ DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.h
b/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.h
new file mode 100644
index 0000000000000..833b654ec808c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.h
@@ -0,0 +1,39 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEAGGREGATECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEAGGREGATECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Finds classes that could be aggregates if their trivial constructors
+/// were removed.
+///
+/// A constructor is considered trivial when it simply forwards each parameter
+/// to a member in declaration order and has an empty body. Removing such
+/// constructors enables aggregate initialization, which is often clearer and
+/// supports designated initializers (C++20).
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-aggregate.html
+class UseAggregateCheck : public ClangTidyCheck {
+public:
+ UseAggregateCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEAGGREGATECHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 68bab88146241..7cdeabc1570ee 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,6 +121,13 @@ New checks
``llvm::to_vector(llvm::make_filter_range(...))`` that can be replaced with
``llvm::map_to_vector`` and ``llvm::filter_to_vector``.
+- New :doc:`modernize-use-aggregate
+ <clang-tidy/checks/modernize/use-aggregate>` check.
+
+ Finds classes that could be aggregates if their trivial forwarding
+ constructors were removed, enabling aggregate initialization and
+ designated initializers.
+
- New :doc:`modernize-use-string-view
<clang-tidy/checks/modernize/use-string-view>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index c475870ed7b31..1c1d9038cb2c7 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -311,6 +311,7 @@ Clang-Tidy Checks
:doc:`modernize-shrink-to-fit <modernize/shrink-to-fit>`, "Yes"
:doc:`modernize-type-traits <modernize/type-traits>`, "Yes"
:doc:`modernize-unary-static-assert <modernize/unary-static-assert>`, "Yes"
+ :doc:`modernize-use-aggregate <modernize/use-aggregate>`,
:doc:`modernize-use-auto <modernize/use-auto>`, "Yes"
:doc:`modernize-use-bool-literals <modernize/use-bool-literals>`, "Yes"
:doc:`modernize-use-constraints <modernize/use-constraints>`, "Yes"
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-aggregate.rst
b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-aggregate.rst
new file mode 100644
index 0000000000000..4effe372d9bc1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-aggregate.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - modernize-use-aggregate
+
+modernize-use-aggregate
+=======================
+
+Finds classes and structs that could be aggregates if their trivial
+forwarding constructors were removed.
+
+A constructor is considered a trivial forwarder when it takes one
+parameter per non-static data member, initializes each member from the
+corresponding parameter in declaration order, and has an empty body.
+Removing such constructors enables aggregate initialization and, in
+C++20, designated initializers.
+
+.. code-block:: c++
+
+ // Before
+ struct Point {
+ int X;
+ int Y;
+ Point(int X, int Y) : X(X), Y(Y) {}
+ };
+
+ Point p(1, 2);
+
+ // After -- remove the constructor
+ struct Point {
+ int X;
+ int Y;
+ };
+
+ Point p{1, 2}; // aggregate initialization
+ Point q{.X = 1, .Y = 2}; // designated initializers (C++20)
+
+The check will not flag a class if it:
+
+- has virtual functions,
+- has private or protected non-static data members,
+- has virtual, private, or protected base classes,
+- has base classes (before C++17),
+- has a user-provided destructor,
+- has additional non-trivial constructors, or
+- is a template specialization.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-aggregate.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-aggregate.cpp
new file mode 100644
index 0000000000000..f34366747795d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-aggregate.cpp
@@ -0,0 +1,145 @@
+// RUN: %check_clang_tidy %s modernize-use-aggregate %t
+
+// Positive: simple forwarding constructor.
+struct Point {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'Point' can be an aggregate type
if the forwarding constructor is removed [modernize-use-aggregate]
+ int X;
+ int Y;
+ Point(int X, int Y) : X(X), Y(Y) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: remove this constructor to enable
aggregate initialization
+};
+
+// Positive: forwarding constructor with class-type member (copy/move).
+namespace std {
+template <typename T>
+class basic_string {
+public:
+ basic_string();
+ basic_string(const basic_string &);
+ basic_string(basic_string &&);
+ basic_string(const char *);
+};
+using string = basic_string<char>;
+} // namespace std
+
+struct Person {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'Person' can be an aggregate
type if the forwarding constructor is removed [modernize-use-aggregate]
+ std::string Name;
+ int Age;
+ Person(std::string Name, int Age) : Name(Name), Age(Age) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: remove this constructor to enable
aggregate initialization
+};
+
+// Negative: constructor does more than forward.
+struct WithLogic {
+ int X;
+ int Y;
+ WithLogic(int X, int Y) : X(X), Y(Y + 1) {}
+};
+
+// Negative: constructor body is not empty.
+struct WithBody {
+ int X;
+ int Y;
+ WithBody(int X, int Y) : X(X), Y(Y) { X++; }
+};
+
+// Negative: has virtual functions.
+struct WithVirtual {
+ int X;
+ virtual void foo();
+ WithVirtual(int X) : X(X) {}
+};
+
+// Negative: has private data members.
+class WithPrivate {
+ int X;
+public:
+ WithPrivate(int X) : X(X) {}
+};
+
+// Negative: has protected data members.
+struct WithProtected {
+protected:
+ int X;
+public:
+ WithProtected(int X) : X(X) {}
+};
+
+// Negative: wrong parameter count.
+struct WrongParamCount {
+ int X;
+ int Y;
+ WrongParamCount(int X) : X(X), Y(0) {}
+};
+
+// Negative: wrong init order (param 1 -> field 0, param 0 -> field 1).
+struct WrongOrder {
+ int X;
+ int Y;
+ WrongOrder(int A, int B) : X(B), Y(A) {}
+};
+
+// Negative: has non-trivial destructor.
+struct WithDestructor {
+ int X;
+ WithDestructor(int X) : X(X) {}
+ ~WithDestructor() {}
+};
+
+// Negative: has additional non-trivial constructor.
+struct MultiCtor {
+ int X;
+ MultiCtor(int X) : X(X) {}
+ MultiCtor(double) : X(0) {}
+};
+
+// Negative: empty struct (already an aggregate).
+struct Empty {
+ Empty() {}
+};
+
+// Negative: template specialization.
+template <typename T>
+struct Templated {
+ T X;
+ Templated(T X) : X(X) {}
+};
+Templated<int> TI(1);
+
+// Negative: virtual base class.
+struct Base {};
+struct WithVirtualBase : virtual Base {
+ int X;
+ WithVirtualBase(int X) : X(X) {}
+};
+
+// Negative: private base class.
+struct WithPrivateBase : private Base {
+ int X;
+ WithPrivateBase(int X) : X(X) {}
+};
+
+// Positive: three fields.
+struct Triple {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'Triple' can be an aggregate
type if the forwarding constructor is removed [modernize-use-aggregate]
+ int A;
+ int B;
+ int C;
+ Triple(int A, int B, int C) : A(A), B(B), C(C) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: remove this constructor to enable
aggregate initialization
+};
+
+// Positive: single field.
+struct Single {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'Single' can be an aggregate
type if the forwarding constructor is removed [modernize-use-aggregate]
+ int X;
+ Single(int X) : X(X) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: note: remove this constructor to enable
aggregate initialization
+};
+
+// Negative: has base class initializer.
+struct Derived : Base {
+ int X;
+ Derived(int X) : Base(), X(X) {}
+};
``````````
</details>
https://github.com/llvm/llvm-project/pull/182061
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
