https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/182061
>From 1c11fa8c6a42d64ea78711b7be9ab92ec76c82f1 Mon Sep 17 00:00:00 2001 From: Helmut Januschka <[email protected]> Date: Wed, 18 Feb 2026 17:53:14 +0100 Subject: [PATCH] [clang-tidy] Add modernize-use-aggregate check 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. For example: ```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) ``` --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 2 + .../modernize/UseAggregateCheck.cpp | 212 ++++++++++++++++++ .../clang-tidy/modernize/UseAggregateCheck.h | 39 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 7 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-aggregate.rst | 43 ++++ .../checkers/modernize/use-aggregate.cpp | 145 ++++++++++++ 8 files changed, 450 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-aggregate.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-aggregate.cpp 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..40f57f86431e1 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -31,6 +31,7 @@ #include "ShrinkToFitCheck.h" #include "TypeTraitsCheck.h" #include "UnaryStaticAssertCheck.h" +#include "UseAggregateCheck.h" #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" #include "UseConstraintsCheck.h" @@ -116,6 +117,7 @@ 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..78048019bd16d --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseAggregateCheck.cpp @@ -0,0 +1,212 @@ +//===----------------------------------------------------------------------===// +// +// 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) {} +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
