Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>, Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/80...@github.com>
llvmbot wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Danny Mösch (SimplyDanny) <details> <summary>Changes</summary> Resolves #<!-- -->77618. --- Full diff: https://github.com/llvm/llvm-project/pull/80541.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/UseDesignatedInitializersCheck.cpp (+77) - (added) clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h (+42) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst (+44) - (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp (+48) ``````````diff diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 28ca52f46943a..6852db6c2ee31 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 UseBoolLiteralsCheck.cpp UseConstraintsCheck.cpp UseDefaultMemberInitCheck.cpp + UseDesignatedInitializersCheck.cpp UseEmplaceCheck.cpp UseEqualsDefaultCheck.cpp UseEqualsDeleteCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 654f4bd0c6ba4..e96cf274f58cf 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -32,6 +32,7 @@ #include "UseBoolLiteralsCheck.h" #include "UseConstraintsCheck.h" #include "UseDefaultMemberInitCheck.h" +#include "UseDesignatedInitializersCheck.h" #include "UseEmplaceCheck.h" #include "UseEqualsDefaultCheck.h" #include "UseEqualsDeleteCheck.h" @@ -68,6 +69,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared"); CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique"); CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value"); + CheckFactories.registerCheck<UseDesignatedInitializersCheck>( + "modernize-use-designated-initializers"); CheckFactories.registerCheck<UseStartsEndsWithCheck>( "modernize-use-starts-ends-with"); CheckFactories.registerCheck<UseStdNumbersCheck>( diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp new file mode 100644 index 0000000000000..d269cef13e6aa --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -0,0 +1,77 @@ +//===--- UseDesignatedInitializersCheck.cpp - clang-tidy ------------------===// +// +// 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 "UseDesignatedInitializersCheck.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include <algorithm> +#include <iterator> +#include <vector> + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static constexpr auto IgnoreSingleElementAggregatesName = + "IgnoreSingleElementAggregates"; +static constexpr auto IgnoreSingleElementAggregatesDefault = true; + +static std::vector<Stmt *> +getUndesignatedComponents(const InitListExpr *SyntacticInitList) { + std::vector<Stmt *> Result; + std::copy_if(SyntacticInitList->begin(), SyntacticInitList->end(), + std::back_inserter(Result), + [](auto S) { return !isa<DesignatedInitExpr>(S); }); + return Result; +} + +UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreSingleElementAggregates( + Options.getLocalOrGlobal(IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregatesDefault)) {} + +void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + initListExpr(hasType(recordDecl().bind("type"))).bind("init"), this); +} + +void UseDesignatedInitializersCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *InitList = Result.Nodes.getNodeAs<InitListExpr>("init"); + const auto *Type = Result.Nodes.getNodeAs<CXXRecordDecl>("type"); + if (!Type || !InitList || !Type->isAggregate()) + return; + if (IgnoreSingleElementAggregates && InitList->getNumInits() == 1) + return; + if (const auto *SyntacticInitList = InitList->getSyntacticForm()) { + const auto UndesignatedComponents = + getUndesignatedComponents(SyntacticInitList); + if (UndesignatedComponents.empty()) + return; + if (UndesignatedComponents.size() == SyntacticInitList->getNumInits()) { + diag(InitList->getLBraceLoc(), "use designated initializer list"); + return; + } + for (const auto *InitExpr : UndesignatedComponents) { + diag(InitExpr->getBeginLoc(), "use designated init expression"); + } + } +} + +void UseDesignatedInitializersCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregates); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h new file mode 100644 index 0000000000000..34290aba06fab --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -0,0 +1,42 @@ +//===--- UseDesignatedInitializersCheck.h - clang-tidy ----------*- C++ -*-===// +// +// 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_USEDESIGNATEDINITIALIZERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEDESIGNATEDINITIALIZERSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Triggers on initializer lists for aggregate type that could be +/// written as designated initializers instead. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-designated-initializers.html +class UseDesignatedInitializersCheck : public ClangTidyCheck { +public: + UseDesignatedInitializersCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + bool IgnoreSingleElementAggregates; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEDESIGNATEDINITIALIZERSCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 9e819ea34c397..ce86f2ea455ff 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,6 +100,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`modernize-use-designated-initializers + <clang-tidy/checks/modernize/use-designated-initializers>` check. + + Triggers on initializer lists for aggregate type that could be + written as designated initializers instead. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index f40192ed9dea2..5d3b99e0996c2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -287,6 +287,7 @@ Clang-Tidy Checks :doc:`modernize-use-bool-literals <modernize/use-bool-literals>`, "Yes" :doc:`modernize-use-constraints <modernize/use-constraints>`, "Yes" :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes" + :doc:`modernize-use-designated-initializers <modernize/use-designated-initializers>`, "Yes" :doc:`modernize-use-emplace <modernize/use-emplace>`, "Yes" :doc:`modernize-use-equals-default <modernize/use-equals-default>`, "Yes" :doc:`modernize-use-equals-delete <modernize/use-equals-delete>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst new file mode 100644 index 0000000000000..1e182218dbd8e --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst @@ -0,0 +1,44 @@ +.. title:: clang-tidy - modernize-use-designated-initializers + +modernize-use-designated-initializers +===================================== + +Triggers on initializer lists for aggregate type that could be written as +designated initializers instead. + +With plain initializer lists, it is very easy to introduce bugs when adding +new fields in the middle of a struct or class type. The same confusion might +arise when changing the order of fields. + +C++ 20 supports the designated initializer syntax for aggregate types. +By applying it, we can always be sure that aggregates are constructed correctly, +because the every variable being initialized is referenced by name. + +Example: + +.. code-block:: + + struct S { int i, j; }; + +is an aggregate type that should be initialized as + +.. code-block:: + + S s{.i = 1, .j = 2}; + +instead of + +.. code-block:: + + S s{1, 2}; + +which could easily become an issue when ``i`` and ``j`` are swapped in the +declaration of ``S``. + +Options +------- + +.. options:: IgnoreSingleElementAggregates + + The value `false` specifies that even initializers for aggregate types + with only a single element should be checked. The default value is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp new file mode 100644 index 0000000000000..d932e2d5e4863 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp @@ -0,0 +1,48 @@ +// RUN: %check_clang_tidy -std=c++20 %s modernize-use-designated-initializers %t +// RUN: %check_clang_tidy -check-suffixes=,SINGLE-ELEMENT -std=c++20 %s modernize-use-designated-initializers %t \ +// RUN: -- -config="{CheckOptions: [{key: modernize-use-designated-initializers.IgnoreSingleElementAggregates, value: false}]}" \ +// RUN: -- + +struct S1 {}; + +S1 s11{}; +S1 s12 = {}; +S1 s13(); +S1 s14; + +struct S2 { int i, j; }; + +S2 s21{.i=1, .j =2}; + +S2 s22 = {1, 2}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] + +S2 s23{1}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers] + +S2 s24{.i = 1}; + +S2 s25 = {.i=1, 2}; +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use designated init expression [modernize-use-designated-initializers] + +class S3 { + public: + S2 s2; + double d; +}; + +S3 s31 = {.s2 = 1, 2, 3.1}; +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: use designated init expression [modernize-use-designated-initializers] + +S3 s32 = {{.i = 1, 2}}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: use designated init expression [modernize-use-designated-initializers] + +struct S4 { + double d; + private: static int i; +}; + +S4 s41 {2.2}; +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] `````````` </details> https://github.com/llvm/llvm-project/pull/80541 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
