https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/80541
From 528ec390e23e6883356606c9acdba3315d5f59bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 13:13:50 +0100 Subject: [PATCH 01/29] Trigger on variable declarations --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../UseDesignatedInitializersCheck.cpp | 58 +++++++++++++++++++ .../UseDesignatedInitializersCheck.h | 33 +++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../modernize/use-designated-initializers.rst | 6 ++ .../modernize/use-designated-initializers.cpp | 32 ++++++++++ 8 files changed, 138 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 28ca52f46943a8..6852db6c2ee311 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 654f4bd0c6ba47..e96cf274f58cfe 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 00000000000000..06f0bb0dc06eac --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -0,0 +1,58 @@ +//===--- 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 std::vector<Stmt *> +getAllUndesignatedInits(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; +} + +void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(varDecl(allOf(has(initListExpr().bind("init")), + hasType(recordDecl().bind("type")))), + 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 (const auto *SyntacticInitList = InitList->getSyntacticForm()) { + const auto UndesignatedParts = getAllUndesignatedInits(SyntacticInitList); + if (UndesignatedParts.empty()) + return; + if (UndesignatedParts.size() == SyntacticInitList->getNumInits()) { + diag(InitList->getLBraceLoc(), "use designated initializer list"); + return; + } + for (const auto *InitExpr : UndesignatedParts) { + diag(InitExpr->getBeginLoc(), "use designated init expression"); + } + } +} + +} // 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 00000000000000..aeaa3f19deb7e8 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -0,0 +1,33 @@ +//===--- 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 { + +/// FIXME: Write a short description. +/// +/// 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) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // 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 ee68c8f49b3df2..4ee1af44b9b37d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -105,6 +105,10 @@ New checks Replaces certain conditional statements with equivalent calls to ``std::min`` or ``std::max``. +- New :doc:`modernize-use-designated-initializers + <clang-tidy/checks/modernize/use-designated-initializers>` check. + + FIXME: add release notes. 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 59ef69f390ee91..5e57bc0ee483fe 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 00000000000000..e0487f9b0e843d --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - modernize-use-designated-initializers + +modernize-use-designated-initializers +===================================== + +FIXME: Describe what patterns does the check detect and why. Give examples. 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 00000000000000..c945454201ee13 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp @@ -0,0 +1,32 @@ +// RUN: %check_clang_tidy %s modernize-use-designated-initializers %t + +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] + +struct S3 { + 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] From ca32ce75d155ca17e8dc237d41047b38afd68f9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 13:34:58 +0100 Subject: [PATCH 02/29] Support nested initializer lists --- .../modernize/UseDesignatedInitializersCheck.cpp | 5 ++--- .../modernize/use-designated-initializers.cpp | 12 ++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 06f0bb0dc06eac..e0400db473db25 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -30,9 +30,8 @@ getAllUndesignatedInits(const InitListExpr *SyntacticInitList) { } void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(varDecl(allOf(has(initListExpr().bind("init")), - hasType(recordDecl().bind("type")))), - this); + Finder->addMatcher( + initListExpr(hasType(recordDecl().bind("type"))).bind("init"), this); } void UseDesignatedInitializersCheck::check( 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 index c945454201ee13..a5eeb5553a9093 100644 --- 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 @@ -30,3 +30,15 @@ struct S3 { 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: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] From 1675bcf2bbc821cbe41f7caec64e8251a11abf51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 13:35:12 +0100 Subject: [PATCH 03/29] Rename --- .../modernize/UseDesignatedInitializersCheck.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index e0400db473db25..7cf6dcb21ac1be 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -21,7 +21,7 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { static std::vector<Stmt *> -getAllUndesignatedInits(const InitListExpr *SyntacticInitList) { +getUndesignatedComponents(const InitListExpr *SyntacticInitList) { std::vector<Stmt *> Result; std::copy_if(SyntacticInitList->begin(), SyntacticInitList->end(), std::back_inserter(Result), @@ -41,14 +41,15 @@ void UseDesignatedInitializersCheck::check( if (!Type || !InitList || !Type->isAggregate()) return; if (const auto *SyntacticInitList = InitList->getSyntacticForm()) { - const auto UndesignatedParts = getAllUndesignatedInits(SyntacticInitList); - if (UndesignatedParts.empty()) + const auto UndesignatedComponents = + getUndesignatedComponents(SyntacticInitList); + if (UndesignatedComponents.empty()) return; - if (UndesignatedParts.size() == SyntacticInitList->getNumInits()) { + if (UndesignatedComponents.size() == SyntacticInitList->getNumInits()) { diag(InitList->getLBraceLoc(), "use designated initializer list"); return; } - for (const auto *InitExpr : UndesignatedParts) { + for (const auto *InitExpr : UndesignatedComponents) { diag(InitExpr->getBeginLoc(), "use designated init expression"); } } From b6a8e57dee25b69bb3a30f952e6eebb5829ddd45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 13:36:10 +0100 Subject: [PATCH 04/29] Enable for C++20 only --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.h | 3 +++ .../checkers/modernize/use-designated-initializers.cpp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index aeaa3f19deb7e8..12a9850ada6a14 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -26,6 +26,9 @@ class UseDesignatedInitializersCheck : public ClangTidyCheck { std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } }; } // namespace clang::tidy::modernize 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 index a5eeb5553a9093..7e41943e884061 100644 --- 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 @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s modernize-use-designated-initializers %t +// RUN: %check_clang_tidy -std=c++20 %s modernize-use-designated-initializers %t struct S1 {}; From b0663cd95a21ec49f09ad339a6086eda9296b800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 13:56:08 +0100 Subject: [PATCH 05/29] Add option to ignore aggregates with single elements --- .../UseDesignatedInitializersCheck.cpp | 19 +++++++++++++++++++ .../UseDesignatedInitializersCheck.h | 9 +++++++-- .../modernize/use-designated-initializers.cpp | 5 ++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 7cf6dcb21ac1be..d269cef13e6aaa 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -20,6 +20,10 @@ 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; @@ -29,6 +33,13 @@ getUndesignatedComponents(const InitListExpr *SyntacticInitList) { 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); @@ -40,6 +51,8 @@ void UseDesignatedInitializersCheck::check( 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); @@ -55,4 +68,10 @@ void UseDesignatedInitializersCheck::check( } } +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 index 12a9850ada6a14..8a60fd7b31c5a4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -19,16 +19,21 @@ namespace clang::tidy::modernize { /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-designated-initializers.html class UseDesignatedInitializersCheck : public ClangTidyCheck { public: - UseDesignatedInitializersCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + 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 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 index 7e41943e884061..350ee03d581ae8 100644 --- 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 @@ -1,4 +1,7 @@ // 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 {}; @@ -41,4 +44,4 @@ struct S4 { }; S4 s41 {2.2}; -// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] From 4aee8852b4c0ce145b2ffbceb33a8b39238d0ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 14:06:17 +0100 Subject: [PATCH 06/29] Test class --- .../checkers/modernize/use-designated-initializers.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 350ee03d581ae8..d932e2d5e4863b 100644 --- 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 @@ -25,7 +25,8 @@ S2 s24{.i = 1}; S2 s25 = {.i=1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use designated init expression [modernize-use-designated-initializers] -struct S3 { +class S3 { + public: S2 s2; double d; }; From 4a0679beccdfe1956074607815e5841ddaf1d8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 14:22:56 +0100 Subject: [PATCH 07/29] Add documentation and release notes --- .../UseDesignatedInitializersCheck.h | 3 +- clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../modernize/use-designated-initializers.rst | 40 ++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index 8a60fd7b31c5a4..34290aba06fab1 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -13,7 +13,8 @@ namespace clang::tidy::modernize { -/// FIXME: Write a short description. +/// 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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4ee1af44b9b37d..2e3a2027cedc38 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -108,7 +108,8 @@ New checks - New :doc:`modernize-use-designated-initializers <clang-tidy/checks/modernize/use-designated-initializers>` check. - FIXME: add release notes. + 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/modernize/use-designated-initializers.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst index e0487f9b0e843d..1e182218dbd8eb 100644 --- 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 @@ -3,4 +3,42 @@ modernize-use-designated-initializers ===================================== -FIXME: Describe what patterns does the check detect and why. Give examples. +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`. From 145c6e5385bd5107b053c6d99cf10ca47eaae203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 14:31:30 +0100 Subject: [PATCH 08/29] Fix typo --- .../clang-tidy/checks/modernize/use-designated-initializers.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 1e182218dbd8eb..27474921a53d5e 100644 --- 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 @@ -38,7 +38,7 @@ declaration of ``S``. Options ------- -.. options:: IgnoreSingleElementAggregates +.. option:: IgnoreSingleElementAggregates The value `false` specifies that even initializers for aggregate types with only a single element should be checked. The default value is `true`. From bcb2da505a8f3c77f82bf7ddd3fbf48f749aac7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 17:24:32 +0100 Subject: [PATCH 09/29] Specify explicit type --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index d269cef13e6aaa..7b23a2e8a09308 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -20,9 +20,9 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { -static constexpr auto IgnoreSingleElementAggregatesName = +static const char *IgnoreSingleElementAggregatesName = "IgnoreSingleElementAggregates"; -static constexpr auto IgnoreSingleElementAggregatesDefault = true; +static const bool IgnoreSingleElementAggregatesDefault = true; static std::vector<Stmt *> getUndesignatedComponents(const InitListExpr *SyntacticInitList) { From 4caec58c15f9eafdf80d49e1d5e6457574f8cb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 17:24:44 +0100 Subject: [PATCH 10/29] Get only local option --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 7b23a2e8a09308..0594f3dfb10306 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -37,8 +37,8 @@ UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoreSingleElementAggregates( - Options.getLocalOrGlobal(IgnoreSingleElementAggregatesName, - IgnoreSingleElementAggregatesDefault)) {} + Options.get(IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregatesDefault)) {} void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( From 909d2bbfa5b019a884a4955bac8aafb41adc3ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 17:25:03 +0100 Subject: [PATCH 11/29] Improve check description --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../clang-tidy/checks/modernize/use-designated-initializers.rst | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index 34290aba06fab1..de06250b0d3d06 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::modernize { -/// Triggers on initializer lists for aggregate type that could be +/// Finds initializer lists for aggregate type that could be /// written as designated initializers instead. /// /// For the user-facing documentation see: diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2e3a2027cedc38..3effe9f9d5b5f0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -108,7 +108,7 @@ 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 + Finds 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/modernize/use-designated-initializers.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst index 27474921a53d5e..0f313eec2b1990 100644 --- 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 @@ -3,7 +3,7 @@ modernize-use-designated-initializers ===================================== -Triggers on initializer lists for aggregate type that could be written as +Finds 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 From 91bf1b6b48323c454f3f10426840446a0e0402c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 18:25:29 +0100 Subject: [PATCH 12/29] Move logic into matcher --- .../UseDesignatedInitializersCheck.cpp | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 0594f3dfb10306..28567e6609fdd3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -12,6 +12,7 @@ #include "clang/AST/Stmt.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" #include <algorithm> #include <iterator> #include <vector> @@ -40,24 +41,32 @@ UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( Options.get(IgnoreSingleElementAggregatesName, IgnoreSingleElementAggregatesDefault)) {} +AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } + +AST_MATCHER(InitListExpr, isFullyDesignated) { + return getUndesignatedComponents(&Node).empty(); +} + +AST_MATCHER(InitListExpr, hasSingleElement) { return Node.getNumInits() == 1; } void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - initListExpr(hasType(recordDecl().bind("type"))).bind("init"), this); + initListExpr(hasType(cxxRecordDecl(isAggregate()).bind("type")), + unless(IgnoreSingleElementAggregates ? hasSingleElement() + : unless(anything())), + unless(isFullyDesignated())) + .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) + if (!Type || !InitList) 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; From 8b99092c44140f83e6d0a801f31f7829871936f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sat, 3 Feb 2024 18:40:45 +0100 Subject: [PATCH 13/29] Add template test --- .../modernize/use-designated-initializers.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 index d932e2d5e4863b..b6f0c3758163d6 100644 --- 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 @@ -46,3 +46,14 @@ struct S4 { S4 s41 {2.2}; // CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] + +S4 s42 = {{}}; +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] + +template<typename S> S template1() { return {10, 11}; } + +S2 s26 = template1<S2>(); + +template<typename S> S template2() { return {}; } + +S2 s27 = template2<S2>(); From 30bfeae261373d30458f24769d6e577edef345a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 4 Feb 2024 11:50:12 +0100 Subject: [PATCH 14/29] Ignore types inheriting fields --- .../modernize/UseDesignatedInitializersCheck.cpp | 15 +++++++++++---- .../modernize/use-designated-initializers.cpp | 4 ++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 28567e6609fdd3..62f2b59241dd70 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -48,12 +48,19 @@ AST_MATCHER(InitListExpr, isFullyDesignated) { } AST_MATCHER(InitListExpr, hasSingleElement) { return Node.getNumInits() == 1; } + +AST_MATCHER_FUNCTION(::internal::Matcher<CXXRecordDecl>, hasBaseWithFields) { + return hasAnyBase(hasType(cxxRecordDecl(has(fieldDecl())))); +} + void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - initListExpr(hasType(cxxRecordDecl(isAggregate()).bind("type")), - unless(IgnoreSingleElementAggregates ? hasSingleElement() - : unless(anything())), - unless(isFullyDesignated())) + initListExpr( + hasType(cxxRecordDecl(isAggregate(), unless(hasBaseWithFields())) + .bind("type")), + unless(IgnoreSingleElementAggregates ? hasSingleElement() + : unless(anything())), + unless(isFullyDesignated())) .bind("init"), this); } 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 index b6f0c3758163d6..0a4a098a7737be 100644 --- 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 @@ -57,3 +57,7 @@ S2 s26 = template1<S2>(); template<typename S> S template2() { return {}; } S2 s27 = template2<S2>(); + +struct S5: S2 { int x, y; }; + +S5 s51 {1, 2, .x = 3, .y = 4}; From 25e62bd11d66d5623479accd62cfddaa4bd1b976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 4 Feb 2024 13:21:26 +0100 Subject: [PATCH 15/29] Ignore types with anonymous fields --- .../UseDesignatedInitializersCheck.cpp | 12 ++++++++++- .../modernize/use-designated-initializers.cpp | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 62f2b59241dd70..84fb59e5a76397 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "UseDesignatedInitializersCheck.h" +#include "clang/AST/APValue.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/Stmt.h" @@ -53,10 +54,19 @@ AST_MATCHER_FUNCTION(::internal::Matcher<CXXRecordDecl>, hasBaseWithFields) { return hasAnyBase(hasType(cxxRecordDecl(has(fieldDecl())))); } +AST_MATCHER(FieldDecl, isAnonymousDecl) { + if (const auto *Record = + Node.getType().getCanonicalType()->getAsRecordDecl()) { + return Record->isAnonymousStructOrUnion() || !Record->getIdentifier(); + } + return false; +} + void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( initListExpr( - hasType(cxxRecordDecl(isAggregate(), unless(hasBaseWithFields())) + hasType(cxxRecordDecl(isAggregate(), unless(hasBaseWithFields()), + unless(has(fieldDecl(isAnonymousDecl())))) .bind("type")), unless(IgnoreSingleElementAggregates ? hasSingleElement() : unless(anything())), 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 index 0a4a098a7737be..46859435c588e4 100644 --- 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 @@ -61,3 +61,23 @@ S2 s27 = template2<S2>(); struct S5: S2 { int x, y; }; S5 s51 {1, 2, .x = 3, .y = 4}; + +struct S6 { + int i; + struct { int j; } s; +}; + +S6 s61 {1, 2}; + +struct S7 { + union { + int k; + double d; + } u; +}; + +S7 s71 {1}; + +struct S8: S7 { int i; }; + +S8 s81{1, 2}; From 728d7763fa125ff54f28bfc6ab4df58aac459029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 4 Feb 2024 13:29:52 +0100 Subject: [PATCH 16/29] Remove restriction to C++ 20 --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.h | 3 --- .../checks/modernize/use-designated-initializers.rst | 6 +++++- .../checkers/modernize/use-designated-initializers.cpp | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index de06250b0d3d06..54dca26c6d735a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -28,9 +28,6 @@ class UseDesignatedInitializersCheck : public ClangTidyCheck { 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: 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 index 0f313eec2b1990..105daf2c50ef2e 100644 --- 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 @@ -12,7 +12,11 @@ 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. +because every variable being initialized is referenced by name. + +Even when compiling in a language version older than C++ 20, depending on you compiler, +designated initializers are potentially supported. Therefore, the check is not restricted +to C++ 20 and older. Example: 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 index 46859435c588e4..9780bf7f0080d0 100644 --- 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 @@ -1,5 +1,5 @@ -// 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: %check_clang_tidy -std=c++17 %s modernize-use-designated-initializers %t +// RUN: %check_clang_tidy -check-suffixes=,SINGLE-ELEMENT -std=c++17 %s modernize-use-designated-initializers %t \ // RUN: -- -config="{CheckOptions: [{key: modernize-use-designated-initializers.IgnoreSingleElementAggregates, value: false}]}" \ // RUN: -- From d5fa47a104bb9c01691137691754c73765922183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 4 Feb 2024 14:05:31 +0100 Subject: [PATCH 17/29] Add option to restrict check to POD types --- .../UseDesignatedInitializersCheck.cpp | 18 +++++++++++++----- .../modernize/UseDesignatedInitializersCheck.h | 1 + .../modernize/use-designated-initializers.rst | 6 ++++++ .../modernize/use-designated-initializers.cpp | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 84fb59e5a76397..b1b44289111c7a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -26,6 +26,9 @@ static const char *IgnoreSingleElementAggregatesName = "IgnoreSingleElementAggregates"; static const bool IgnoreSingleElementAggregatesDefault = true; +static const char *RestrictToPODTypesName = "RestrictToPODTypes"; +static const bool RestrictToPODTypesDefault = false; + static std::vector<Stmt *> getUndesignatedComponents(const InitListExpr *SyntacticInitList) { std::vector<Stmt *> Result; @@ -37,13 +40,16 @@ getUndesignatedComponents(const InitListExpr *SyntacticInitList) { UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IgnoreSingleElementAggregates( - Options.get(IgnoreSingleElementAggregatesName, - IgnoreSingleElementAggregatesDefault)) {} + : ClangTidyCheck(Name, Context), IgnoreSingleElementAggregates(Options.get( + IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregatesDefault)), + RestrictToPODTypes( + Options.get(RestrictToPODTypesName, RestrictToPODTypesDefault)) {} AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } +AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); } + AST_MATCHER(InitListExpr, isFullyDesignated) { return getUndesignatedComponents(&Node).empty(); } @@ -65,7 +71,8 @@ AST_MATCHER(FieldDecl, isAnonymousDecl) { void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( initListExpr( - hasType(cxxRecordDecl(isAggregate(), unless(hasBaseWithFields()), + hasType(cxxRecordDecl(RestrictToPODTypes ? isPOD() : isAggregate(), + unless(hasBaseWithFields()), unless(has(fieldDecl(isAnonymousDecl())))) .bind("type")), unless(IgnoreSingleElementAggregates ? hasSingleElement() @@ -98,6 +105,7 @@ void UseDesignatedInitializersCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, IgnoreSingleElementAggregatesName, IgnoreSingleElementAggregates); + Options.store(Opts, RestrictToPODTypesName, RestrictToPODTypes); } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index 54dca26c6d735a..9aa216d30ef6b6 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -32,6 +32,7 @@ class UseDesignatedInitializersCheck : public ClangTidyCheck { private: bool IgnoreSingleElementAggregates; + bool RestrictToPODTypes; }; } // namespace clang::tidy::modernize 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 index 105daf2c50ef2e..a306e784c33562 100644 --- 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 @@ -46,3 +46,9 @@ Options The value `false` specifies that even initializers for aggregate types with only a single element should be checked. The default value is `true`. + +.. option:: RestrictToPODTypes + + The value `true` specifies that only Plain Old Data (POD) types shall be + checked. This makes the check applicable to even older C++ standards. + The default value is `false`. 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 index 9780bf7f0080d0..e0b722ec091671 100644 --- 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 @@ -2,6 +2,9 @@ // RUN: %check_clang_tidy -check-suffixes=,SINGLE-ELEMENT -std=c++17 %s modernize-use-designated-initializers %t \ // RUN: -- -config="{CheckOptions: [{key: modernize-use-designated-initializers.IgnoreSingleElementAggregates, value: false}]}" \ // RUN: -- +// RUN: %check_clang_tidy -check-suffixes=POD -std=c++17 %s modernize-use-designated-initializers %t \ +// RUN: -- -config="{CheckOptions: [{key: modernize-use-designated-initializers.RestrictToPODTypes, value: true}]}" \ +// RUN: -- struct S1 {}; @@ -16,14 +19,17 @@ S2 s21{.i=1, .j =2}; S2 s22 = {1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-2]]: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] +// CHECK-MESSAGES-POD: :[[@LINE-2]]: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] +// CHECK-MESSAGES-POD: :[[@LINE-2]]:17: warning: use designated init expression [modernize-use-designated-initializers] class S3 { public: @@ -34,10 +40,14 @@ class S3 { 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] +// CHECK-MESSAGES-POD: :[[@LINE-3]]:20: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-4]]: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] +// CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-4]]:20: warning: use designated init expression [modernize-use-designated-initializers] struct S4 { double d; @@ -81,3 +91,11 @@ S7 s71 {1}; struct S8: S7 { int i; }; S8 s81{1, 2}; + +struct S9 { + int i, j; + S9 &operator=(S9); +}; + +S9 s91{1, 2}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers] From 189e5bb867f60d6eff80fa73b8276098fb27ec3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 11:52:29 +0100 Subject: [PATCH 18/29] Add fix-its --- .../modernize/UseDesignatedInitializersCheck.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index b1b44289111c7a..081286081b4cbf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -95,8 +95,19 @@ void UseDesignatedInitializersCheck::check( diag(InitList->getLBraceLoc(), "use designated initializer list"); return; } - for (const auto *InitExpr : UndesignatedComponents) { - diag(InitExpr->getBeginLoc(), "use designated init expression"); + const auto FieldIterator = Type->fields().begin(); + for (const auto *InitExpr : *SyntacticInitList) { + const auto Field = std::next(FieldIterator); + if (std::find(UndesignatedComponents.begin(), + UndesignatedComponents.end(), + InitExpr) == UndesignatedComponents.end()) + continue; + if (const auto *FieldID = Field->getIdentifier()) { + const auto FieldName = FieldID->getName(); + diag(InitExpr->getBeginLoc(), "use designated init expression") + << FixItHint::CreateInsertion(InitExpr->getBeginLoc(), + "." + FieldName.str() + "="); + } } } } From 9c53c3cbada877aec866cb4292f426fa1700eafb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 14:36:19 +0100 Subject: [PATCH 19/29] Move computation of designated initializers to common place --- clang-tools-extra/clangd/InlayHints.cpp | 167 +-------------- .../clang/Tooling/DesignatedInitializers.h | 43 ++++ clang/lib/Tooling/CMakeLists.txt | 1 + clang/lib/Tooling/DesignatedInitializers.cpp | 195 ++++++++++++++++++ 4 files changed, 241 insertions(+), 165 deletions(-) create mode 100644 clang/include/clang/Tooling/DesignatedInitializers.h create mode 100644 clang/lib/Tooling/DesignatedInitializers.cpp diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 671a9c30ffa959..09f5dba5697b10 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -23,8 +23,8 @@ #include "clang/Basic/Builtins.h" #include "clang/Basic/OperatorKinds.h" #include "clang/Basic/SourceManager.h" +#include "clang/Tooling/DesignatedInitializers.h" #include "llvm/ADT/DenseSet.h" -#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" @@ -42,169 +42,6 @@ namespace { // For now, inlay hints are always anchored at the left or right of their range. enum class HintSide { Left, Right }; -// Helper class to iterate over the designator names of an aggregate type. -// -// For an array type, yields [0], [1], [2]... -// For aggregate classes, yields null for each base, then .field1, .field2, ... -class AggregateDesignatorNames { -public: - AggregateDesignatorNames(QualType T) { - if (!T.isNull()) { - T = T.getCanonicalType(); - if (T->isArrayType()) { - IsArray = true; - Valid = true; - return; - } - if (const RecordDecl *RD = T->getAsRecordDecl()) { - Valid = true; - FieldsIt = RD->field_begin(); - FieldsEnd = RD->field_end(); - if (const auto *CRD = llvm::dyn_cast<CXXRecordDecl>(RD)) { - BasesIt = CRD->bases_begin(); - BasesEnd = CRD->bases_end(); - Valid = CRD->isAggregate(); - } - OneField = Valid && BasesIt == BasesEnd && FieldsIt != FieldsEnd && - std::next(FieldsIt) == FieldsEnd; - } - } - } - // Returns false if the type was not an aggregate. - operator bool() { return Valid; } - // Advance to the next element in the aggregate. - void next() { - if (IsArray) - ++Index; - else if (BasesIt != BasesEnd) - ++BasesIt; - else if (FieldsIt != FieldsEnd) - ++FieldsIt; - } - // Print the designator to Out. - // Returns false if we could not produce a designator for this element. - bool append(std::string &Out, bool ForSubobject) { - if (IsArray) { - Out.push_back('['); - Out.append(std::to_string(Index)); - Out.push_back(']'); - return true; - } - if (BasesIt != BasesEnd) - return false; // Bases can't be designated. Should we make one up? - if (FieldsIt != FieldsEnd) { - llvm::StringRef FieldName; - if (const IdentifierInfo *II = FieldsIt->getIdentifier()) - FieldName = II->getName(); - - // For certain objects, their subobjects may be named directly. - if (ForSubobject && - (FieldsIt->isAnonymousStructOrUnion() || - // std::array<int,3> x = {1,2,3}. Designators not strictly valid! - (OneField && isReservedName(FieldName)))) - return true; - - if (!FieldName.empty() && !isReservedName(FieldName)) { - Out.push_back('.'); - Out.append(FieldName.begin(), FieldName.end()); - return true; - } - return false; - } - return false; - } - -private: - bool Valid = false; - bool IsArray = false; - bool OneField = false; // e.g. std::array { T __elements[N]; } - unsigned Index = 0; - CXXRecordDecl::base_class_const_iterator BasesIt; - CXXRecordDecl::base_class_const_iterator BasesEnd; - RecordDecl::field_iterator FieldsIt; - RecordDecl::field_iterator FieldsEnd; -}; - -// Collect designator labels describing the elements of an init list. -// -// This function contributes the designators of some (sub)object, which is -// represented by the semantic InitListExpr Sem. -// This includes any nested subobjects, but *only* if they are part of the same -// original syntactic init list (due to brace elision). -// In other words, it may descend into subobjects but not written init-lists. -// -// For example: struct Outer { Inner a,b; }; struct Inner { int x, y; } -// Outer o{{1, 2}, 3}; -// This function will be called with Sem = { {1, 2}, {3, ImplicitValue} } -// It should generate designators '.a:' and '.b.x:'. -// '.a:' is produced directly without recursing into the written sublist. -// (The written sublist will have a separate collectDesignators() call later). -// Recursion with Prefix='.b' and Sem = {3, ImplicitValue} produces '.b.x:'. -void collectDesignators(const InitListExpr *Sem, - llvm::DenseMap<SourceLocation, std::string> &Out, - const llvm::DenseSet<SourceLocation> &NestedBraces, - std::string &Prefix) { - if (!Sem || Sem->isTransparent()) - return; - assert(Sem->isSemanticForm()); - - // The elements of the semantic form all correspond to direct subobjects of - // the aggregate type. `Fields` iterates over these subobject names. - AggregateDesignatorNames Fields(Sem->getType()); - if (!Fields) - return; - for (const Expr *Init : Sem->inits()) { - auto Next = llvm::make_scope_exit([&, Size(Prefix.size())] { - Fields.next(); // Always advance to the next subobject name. - Prefix.resize(Size); // Erase any designator we appended. - }); - // Skip for a broken initializer or if it is a "hole" in a subobject that - // was not explicitly initialized. - if (!Init || llvm::isa<ImplicitValueInitExpr>(Init)) - continue; - - const auto *BraceElidedSubobject = llvm::dyn_cast<InitListExpr>(Init); - if (BraceElidedSubobject && - NestedBraces.contains(BraceElidedSubobject->getLBraceLoc())) - BraceElidedSubobject = nullptr; // there were braces! - - if (!Fields.append(Prefix, BraceElidedSubobject != nullptr)) - continue; // no designator available for this subobject - if (BraceElidedSubobject) { - // If the braces were elided, this aggregate subobject is initialized - // inline in the same syntactic list. - // Descend into the semantic list describing the subobject. - // (NestedBraces are still correct, they're from the same syntactic list). - collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix); - continue; - } - Out.try_emplace(Init->getBeginLoc(), Prefix); - } -} - -// Get designators describing the elements of a (syntactic) init list. -// This does not produce designators for any explicitly-written nested lists. -llvm::DenseMap<SourceLocation, std::string> -getDesignators(const InitListExpr *Syn) { - assert(Syn->isSyntacticForm()); - - // collectDesignators needs to know which InitListExprs in the semantic tree - // were actually written, but InitListExpr::isExplicit() lies. - // Instead, record where braces of sub-init-lists occur in the syntactic form. - llvm::DenseSet<SourceLocation> NestedBraces; - for (const Expr *Init : Syn->inits()) - if (auto *Nested = llvm::dyn_cast<InitListExpr>(Init)) - NestedBraces.insert(Nested->getLBraceLoc()); - - // Traverse the semantic form to find the designators. - // We use their SourceLocation to correlate with the syntactic form later. - llvm::DenseMap<SourceLocation, std::string> Designators; - std::string EmptyPrefix; - collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(), - Designators, NestedBraces, EmptyPrefix); - return Designators; -} - void stripLeadingUnderscores(StringRef &Name) { Name = Name.ltrim('_'); } // getDeclForType() returns the decl responsible for Type's spelling. @@ -854,7 +691,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { if (Syn->isIdiomaticZeroInitializer(AST.getLangOpts())) return true; llvm::DenseMap<SourceLocation, std::string> Designators = - getDesignators(Syn); + clang::tooling::getDesignators(Syn); for (const Expr *Init : Syn->inits()) { if (llvm::isa<DesignatedInitExpr>(Init)) continue; diff --git a/clang/include/clang/Tooling/DesignatedInitializers.h b/clang/include/clang/Tooling/DesignatedInitializers.h new file mode 100644 index 00000000000000..e978dfe639f0c0 --- /dev/null +++ b/clang/include/clang/Tooling/DesignatedInitializers.h @@ -0,0 +1,43 @@ +//===--- DesignatedInitializers.h -------------------------------*- 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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides utilities for designated initializers. +/// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Expr.h" +#include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/DenseMap.h" + +namespace clang::tooling { + +/// Get designators describing the elements of a (syntactic) init list. +/// +/// Given for example the type +/// +/// struct S { int i, j; }; +/// +/// and the definition +/// +/// S s{1, 2}; +/// +/// calling `getDesignators` for the initializer list expression `{1, 2}` +/// would produce the map `{loc(1): ".i", loc(2): ".j"}`. +/// +/// It does not produce designators for any explicitly-written nested lists, +/// e.g. `{1, .j=2}` would only return `{loc(1): ".i"}`. +/// +/// It also considers structs with fields of record types like +/// `struct T { S s; };`. In this case, there would be designators of the +/// form +/// `.s.i` and `.s.j` in the returned map. +llvm::DenseMap<clang::SourceLocation, std::string> +getDesignators(const clang::InitListExpr *Syn); + +} // namespace clang::tooling diff --git a/clang/lib/Tooling/CMakeLists.txt b/clang/lib/Tooling/CMakeLists.txt index aff39e4de13c0b..8b0853d9150bb3 100644 --- a/clang/lib/Tooling/CMakeLists.txt +++ b/clang/lib/Tooling/CMakeLists.txt @@ -106,6 +106,7 @@ add_clang_library(clangTooling ArgumentsAdjusters.cpp CommonOptionsParser.cpp CompilationDatabase.cpp + DesignatedInitializers.cpp Execution.cpp ExpandResponseFilesCompilationDatabase.cpp FileMatchTrie.cpp diff --git a/clang/lib/Tooling/DesignatedInitializers.cpp b/clang/lib/Tooling/DesignatedInitializers.cpp new file mode 100644 index 00000000000000..8671625bd19ea3 --- /dev/null +++ b/clang/lib/Tooling/DesignatedInitializers.cpp @@ -0,0 +1,195 @@ +//===--- DesignatedInitializers.cpp ---------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides utilities for designated initializers. +/// +//===----------------------------------------------------------------------===// + +#include "clang/Tooling/DesignatedInitializers.h" +#include "clang/AST/DeclCXX.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/ScopeExit.h" + +namespace clang::tooling { + +namespace { + +/// Returns true if Name is reserved, like _Foo or __Vector_base. +static inline bool isReservedName(llvm::StringRef Name) { + // This doesn't catch all cases, but the most common. + return Name.size() >= 2 && Name[0] == '_' && + (isUppercase(Name[1]) || Name[1] == '_'); +} + +// Helper class to iterate over the designator names of an aggregate type. +// +// For an array type, yields [0], [1], [2]... +// For aggregate classes, yields null for each base, then .field1, .field2, +// ... +class AggregateDesignatorNames { +public: + AggregateDesignatorNames(QualType T) { + if (!T.isNull()) { + T = T.getCanonicalType(); + if (T->isArrayType()) { + IsArray = true; + Valid = true; + return; + } + if (const RecordDecl *RD = T->getAsRecordDecl()) { + Valid = true; + FieldsIt = RD->field_begin(); + FieldsEnd = RD->field_end(); + if (const auto *CRD = llvm::dyn_cast<CXXRecordDecl>(RD)) { + BasesIt = CRD->bases_begin(); + BasesEnd = CRD->bases_end(); + Valid = CRD->isAggregate(); + } + OneField = Valid && BasesIt == BasesEnd && FieldsIt != FieldsEnd && + std::next(FieldsIt) == FieldsEnd; + } + } + } + // Returns false if the type was not an aggregate. + operator bool() { return Valid; } + // Advance to the next element in the aggregate. + void next() { + if (IsArray) + ++Index; + else if (BasesIt != BasesEnd) + ++BasesIt; + else if (FieldsIt != FieldsEnd) + ++FieldsIt; + } + // Print the designator to Out. + // Returns false if we could not produce a designator for this element. + bool append(std::string &Out, bool ForSubobject) { + if (IsArray) { + Out.push_back('['); + Out.append(std::to_string(Index)); + Out.push_back(']'); + return true; + } + if (BasesIt != BasesEnd) + return false; // Bases can't be designated. Should we make one up? + if (FieldsIt != FieldsEnd) { + llvm::StringRef FieldName; + if (const IdentifierInfo *II = FieldsIt->getIdentifier()) + FieldName = II->getName(); + + // For certain objects, their subobjects may be named directly. + if (ForSubobject && + (FieldsIt->isAnonymousStructOrUnion() || + // std::array<int,3> x = {1,2,3}. Designators not strictly valid! + (OneField && isReservedName(FieldName)))) + return true; + + if (!FieldName.empty() && !isReservedName(FieldName)) { + Out.push_back('.'); + Out.append(FieldName.begin(), FieldName.end()); + return true; + } + return false; + } + return false; + } + +private: + bool Valid = false; + bool IsArray = false; + bool OneField = false; // e.g. std::array { T __elements[N]; } + unsigned Index = 0; + CXXRecordDecl::base_class_const_iterator BasesIt; + CXXRecordDecl::base_class_const_iterator BasesEnd; + RecordDecl::field_iterator FieldsIt; + RecordDecl::field_iterator FieldsEnd; +}; + +// Collect designator labels describing the elements of an init list. +// +// This function contributes the designators of some (sub)object, which is +// represented by the semantic InitListExpr Sem. +// This includes any nested subobjects, but *only* if they are part of the +// same original syntactic init list (due to brace elision). In other words, +// it may descend into subobjects but not written init-lists. +// +// For example: struct Outer { Inner a,b; }; struct Inner { int x, y; } +// Outer o{{1, 2}, 3}; +// This function will be called with Sem = { {1, 2}, {3, ImplicitValue} } +// It should generate designators '.a:' and '.b.x:'. +// '.a:' is produced directly without recursing into the written sublist. +// (The written sublist will have a separate collectDesignators() call later). +// Recursion with Prefix='.b' and Sem = {3, ImplicitValue} produces '.b.x:'. +void collectDesignators(const InitListExpr *Sem, + llvm::DenseMap<SourceLocation, std::string> &Out, + const llvm::DenseSet<SourceLocation> &NestedBraces, + std::string &Prefix) { + if (!Sem || Sem->isTransparent()) + return; + assert(Sem->isSemanticForm()); + + // The elements of the semantic form all correspond to direct subobjects of + // the aggregate type. `Fields` iterates over these subobject names. + AggregateDesignatorNames Fields(Sem->getType()); + if (!Fields) + return; + for (const Expr *Init : Sem->inits()) { + auto Next = llvm::make_scope_exit([&, Size(Prefix.size())] { + Fields.next(); // Always advance to the next subobject name. + Prefix.resize(Size); // Erase any designator we appended. + }); + // Skip for a broken initializer or if it is a "hole" in a subobject that + // was not explicitly initialized. + if (!Init || llvm::isa<ImplicitValueInitExpr>(Init)) + continue; + + const auto *BraceElidedSubobject = llvm::dyn_cast<InitListExpr>(Init); + if (BraceElidedSubobject && + NestedBraces.contains(BraceElidedSubobject->getLBraceLoc())) + BraceElidedSubobject = nullptr; // there were braces! + + if (!Fields.append(Prefix, BraceElidedSubobject != nullptr)) + continue; // no designator available for this subobject + if (BraceElidedSubobject) { + // If the braces were elided, this aggregate subobject is initialized + // inline in the same syntactic list. + // Descend into the semantic list describing the subobject. + // (NestedBraces are still correct, they're from the same syntactic + // list). + collectDesignators(BraceElidedSubobject, Out, NestedBraces, Prefix); + continue; + } + Out.try_emplace(Init->getBeginLoc(), Prefix); + } +} + +} // namespace + +llvm::DenseMap<SourceLocation, std::string> +getDesignators(const InitListExpr *Syn) { + assert(Syn->isSyntacticForm()); + + // collectDesignators needs to know which InitListExprs in the semantic tree + // were actually written, but InitListExpr::isExplicit() lies. + // Instead, record where braces of sub-init-lists occur in the syntactic form. + llvm::DenseSet<SourceLocation> NestedBraces; + for (const Expr *Init : Syn->inits()) + if (auto *Nested = llvm::dyn_cast<InitListExpr>(Init)) + NestedBraces.insert(Nested->getLBraceLoc()); + + // Traverse the semantic form to find the designators. + // We use their SourceLocation to correlate with the syntactic form later. + llvm::DenseMap<SourceLocation, std::string> Designators; + std::string EmptyPrefix; + collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(), + Designators, NestedBraces, EmptyPrefix); + return Designators; +} + +} // namespace clang::tooling From 42b9448bc25220dd4c8b544c02648fea10ddd87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 14:37:45 +0100 Subject: [PATCH 20/29] Fully implement fix-its reusing designated initializer computation from Clangd --- .../UseDesignatedInitializersCheck.cpp | 67 ++++++++++--------- .../modernize/use-designated-initializers.cpp | 8 +++ 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 081286081b4cbf..9e90df711df66a 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -14,9 +14,9 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchersMacros.h" -#include <algorithm> -#include <iterator> -#include <vector> +#include "clang/Basic/Diagnostic.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/DesignatedInitializers.h" using namespace clang::ast_matchers; @@ -29,15 +29,6 @@ static const bool IgnoreSingleElementAggregatesDefault = true; static const char *RestrictToPODTypesName = "RestrictToPODTypes"; static const bool RestrictToPODTypesDefault = false; -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.get( @@ -51,7 +42,9 @@ AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); } AST_MATCHER(InitListExpr, isFullyDesignated) { - return getUndesignatedComponents(&Node).empty(); + return std::all_of(Node.begin(), Node.end(), [](auto *InitExpr) { + return isa<DesignatedInitExpr>(InitExpr); + }); } AST_MATCHER(InitListExpr, hasSingleElement) { return Node.getNumInits() == 1; } @@ -82,6 +75,12 @@ void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { this); } +static bool isFullyUndesignated(const InitListExpr *SyntacticInitList) { + return std::all_of( + SyntacticInitList->begin(), SyntacticInitList->end(), + [](auto *InitExpr) { return !isa<DesignatedInitExpr>(InitExpr); }); +} + void UseDesignatedInitializersCheck::check( const MatchFinder::MatchResult &Result) { const auto *InitList = Result.Nodes.getNodeAs<InitListExpr>("init"); @@ -89,24 +88,30 @@ void UseDesignatedInitializersCheck::check( if (!Type || !InitList) return; if (const auto *SyntacticInitList = InitList->getSyntacticForm()) { - const auto UndesignatedComponents = - getUndesignatedComponents(SyntacticInitList); - if (UndesignatedComponents.size() == SyntacticInitList->getNumInits()) { - diag(InitList->getLBraceLoc(), "use designated initializer list"); - return; - } - const auto FieldIterator = Type->fields().begin(); - for (const auto *InitExpr : *SyntacticInitList) { - const auto Field = std::next(FieldIterator); - if (std::find(UndesignatedComponents.begin(), - UndesignatedComponents.end(), - InitExpr) == UndesignatedComponents.end()) - continue; - if (const auto *FieldID = Field->getIdentifier()) { - const auto FieldName = FieldID->getName(); - diag(InitExpr->getBeginLoc(), "use designated init expression") - << FixItHint::CreateInsertion(InitExpr->getBeginLoc(), - "." + FieldName.str() + "="); + const llvm::DenseMap<clang::SourceLocation, std::string> Designators = + clang::tooling::getDesignators(SyntacticInitList); + if (isFullyUndesignated(SyntacticInitList)) { + std::string NewList = "{"; + for (const Stmt *InitExpr : *SyntacticInitList) { + if (InitExpr != *SyntacticInitList->begin()) + NewList += ", "; + NewList += Designators.at(InitExpr->getBeginLoc()); + NewList += "="; + NewList += Lexer::getSourceText( + CharSourceRange::getTokenRange(InitExpr->getSourceRange()), + *Result.SourceManager, getLangOpts()); + } + NewList += "}"; + diag(InitList->getLBraceLoc(), "use designated initializer list") + << FixItHint::CreateReplacement(InitList->getSourceRange(), NewList); + } else { + for (const auto *InitExpr : *SyntacticInitList) { + if (!isa<DesignatedInitExpr>(InitExpr)) { + diag(InitExpr->getBeginLoc(), "use designated init expression") + << FixItHint::CreateInsertion( + InitExpr->getBeginLoc(), + Designators.at(InitExpr->getBeginLoc()) + "="); + } } } } 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 index e0b722ec091671..7d8c2ee63b4aef 100644 --- 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 @@ -20,16 +20,19 @@ S2 s21{.i=1, .j =2}; S2 s22 = {1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-2]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES: S2 s22 = {.i=1, .j=2}; S2 s23{1}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-2]]:7: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES: S2 s23{.i=1}; S2 s24{.i = 1}; S2 s25 = {.i=1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use designated init expression [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-2]]:17: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S2 s25 = {.i=1, .j=2}; class S3 { public: @@ -42,12 +45,14 @@ S3 s31 = {.s2 = 1, 2, 3.1}; // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: use designated init expression [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-3]]:20: warning: use designated init expression [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-4]]:23: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S3 s31 = {.s2 = 1, .s2.j=2, .d=3.1}; 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] // CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-4]]:20: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S3 s32 = {.s2={.i = 1, 2}}; struct S4 { double d; @@ -56,9 +61,11 @@ struct S4 { S4 s41 {2.2}; // CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES-SINGLE-ELEMENT: S4 s41 {.d=2.2}; S4 s42 = {{}}; // CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES-SINGLE-ELEMENT: S4 s42 = {.d={}}; template<typename S> S template1() { return {10, 11}; } @@ -99,3 +106,4 @@ struct S9 { S9 s91{1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES: S9 s91{.i=1, .j=2}; From c2bb32a2740887b03b1927be997a55cd92ee98cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 14:46:21 +0100 Subject: [PATCH 21/29] Separate by empty line --- clang-tools-extra/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3effe9f9d5b5f0..5d532e3cfb9a9c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -105,6 +105,7 @@ New checks Replaces certain conditional statements with equivalent calls to ``std::min`` or ``std::max``. + - New :doc:`modernize-use-designated-initializers <clang-tidy/checks/modernize/use-designated-initializers>` check. From 606a1b75928cd6f7cfbedf1dcfd3c09eb45e9adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 15:00:18 +0100 Subject: [PATCH 22/29] Add test for default-initialized fields --- .../checkers/modernize/use-designated-initializers.cpp | 6 ++++++ 1 file changed, 6 insertions(+) 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 index 7d8c2ee63b4aef..5eada72e225411 100644 --- 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 @@ -107,3 +107,9 @@ struct S9 { S9 s91{1, 2}; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers] // CHECK-FIXES: S9 s91{.i=1, .j=2}; + +struct S10 { int i = 1, j = 2; }; + +S10 s101 {1, .j=2}; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S10 s101 {.i=1, .j=2}; From c758b459ec9c06d2b32de40af72c8f61ae639fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 11 Feb 2024 17:13:37 +0100 Subject: [PATCH 23/29] Fix C++20 style --- .../checks/modernize/use-designated-initializers.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index a306e784c33562..c17aea84b73941 100644 --- 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 @@ -10,13 +10,13 @@ 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. +C++20 supports the designated initializer syntax for aggregate types. By applying it, we can always be sure that aggregates are constructed correctly, because every variable being initialized is referenced by name. -Even when compiling in a language version older than C++ 20, depending on you compiler, +Even when compiling in a language version older than C++20, depending on you compiler, designated initializers are potentially supported. Therefore, the check is not restricted -to C++ 20 and older. +to C++20 and older. Example: From 5526b961ebce8f4e8dc2bda9c5f2caa40c0cea1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Mon, 12 Feb 2024 19:24:31 +0100 Subject: [PATCH 24/29] Have custom AST matchers in anonymous namespace --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 9e90df711df66a..e780002b6a02ed 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -37,6 +37,8 @@ UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( RestrictToPODTypes( Options.get(RestrictToPODTypesName, RestrictToPODTypesDefault)) {} +namespace { + AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); } @@ -61,6 +63,8 @@ AST_MATCHER(FieldDecl, isAnonymousDecl) { return false; } +} // namespace + void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( initListExpr( From 13865368d09cca423a0ffb3982a8b90a474fcca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Mon, 12 Feb 2024 19:36:39 +0100 Subject: [PATCH 25/29] Keep public methods public --- .../clang-tidy/modernize/UseDesignatedInitializersCheck.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h index 9aa216d30ef6b6..8c947a1027d957 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -23,12 +23,11 @@ class UseDesignatedInitializersCheck : public ClangTidyCheck { UseDesignatedInitializersCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; -private: std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: bool IgnoreSingleElementAggregates; From 6ef3433528370061399532ea954efb67ad088e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Mon, 12 Feb 2024 19:43:37 +0100 Subject: [PATCH 26/29] Rename method --- .../UseDesignatedInitializersCheck.cpp | 2 +- clang-tools-extra/clangd/InlayHints.cpp | 5 +++-- .../clang/Tooling/DesignatedInitializers.h | 21 +++++++++---------- clang/lib/Tooling/DesignatedInitializers.cpp | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index e780002b6a02ed..84b63f8562f480 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -93,7 +93,7 @@ void UseDesignatedInitializersCheck::check( return; if (const auto *SyntacticInitList = InitList->getSyntacticForm()) { const llvm::DenseMap<clang::SourceLocation, std::string> Designators = - clang::tooling::getDesignators(SyntacticInitList); + clang::tooling::getUnwrittenDesignators(SyntacticInitList); if (isFullyUndesignated(SyntacticInitList)) { std::string NewList = "{"; for (const Stmt *InitExpr : *SyntacticInitList) { diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 09f5dba5697b10..c8d3a94affee9d 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -684,14 +684,15 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { // This is the one we will ultimately attach designators to. // It may have subobject initializers inlined without braces. The *semantic* // form of the init-list has nested init-lists for these. - // getDesignators will look at the semantic form to determine the labels. + // getUnwrittenDesignators will look at the semantic form to determine the + // labels. assert(Syn->isSyntacticForm() && "RAV should not visit implicit code!"); if (!Cfg.InlayHints.Designators) return true; if (Syn->isIdiomaticZeroInitializer(AST.getLangOpts())) return true; llvm::DenseMap<SourceLocation, std::string> Designators = - clang::tooling::getDesignators(Syn); + clang::tooling::getUnwrittenDesignators(Syn); for (const Expr *Init : Syn->inits()) { if (llvm::isa<DesignatedInitExpr>(Init)) continue; diff --git a/clang/include/clang/Tooling/DesignatedInitializers.h b/clang/include/clang/Tooling/DesignatedInitializers.h index e978dfe639f0c0..fa1deeae0791fc 100644 --- a/clang/include/clang/Tooling/DesignatedInitializers.h +++ b/clang/include/clang/Tooling/DesignatedInitializers.h @@ -20,24 +20,23 @@ namespace clang::tooling { /// Get designators describing the elements of a (syntactic) init list. /// /// Given for example the type -/// -/// struct S { int i, j; }; -/// +/// \code +/// struct S { int i, j; }; +/// \endcode /// and the definition -/// -/// S s{1, 2}; -/// -/// calling `getDesignators` for the initializer list expression `{1, 2}` -/// would produce the map `{loc(1): ".i", loc(2): ".j"}`. +/// \code +/// S s{1, 2}; +/// \endcode +/// calling `getUnwrittenDesignators` for the initializer list expression +/// `{1, 2}` would produce the map `{loc(1): ".i", loc(2): ".j"}`. /// /// It does not produce designators for any explicitly-written nested lists, /// e.g. `{1, .j=2}` would only return `{loc(1): ".i"}`. /// /// It also considers structs with fields of record types like /// `struct T { S s; };`. In this case, there would be designators of the -/// form -/// `.s.i` and `.s.j` in the returned map. +/// form `.s.i` and `.s.j` in the returned map. llvm::DenseMap<clang::SourceLocation, std::string> -getDesignators(const clang::InitListExpr *Syn); +getUnwrittenDesignators(const clang::InitListExpr *Syn); } // namespace clang::tooling diff --git a/clang/lib/Tooling/DesignatedInitializers.cpp b/clang/lib/Tooling/DesignatedInitializers.cpp index 8671625bd19ea3..9d8d0a5f4795a7 100644 --- a/clang/lib/Tooling/DesignatedInitializers.cpp +++ b/clang/lib/Tooling/DesignatedInitializers.cpp @@ -172,7 +172,7 @@ void collectDesignators(const InitListExpr *Sem, } // namespace llvm::DenseMap<SourceLocation, std::string> -getDesignators(const InitListExpr *Syn) { +getUnwrittenDesignators(const InitListExpr *Syn) { assert(Syn->isSyntacticForm()); // collectDesignators needs to know which InitListExprs in the semantic tree From 6f8d713dff29f4c9fdfa9c27fa210a92456af00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Mon, 12 Feb 2024 22:22:37 +0100 Subject: [PATCH 27/29] Analyze syntactic form only --- .../modernize/UseDesignatedInitializersCheck.cpp | 8 +++++--- .../modernize/use-designated-initializers.cpp | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index 84b63f8562f480..d4cade4a604ae2 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -44,9 +44,11 @@ AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); } AST_MATCHER(InitListExpr, isFullyDesignated) { - return std::all_of(Node.begin(), Node.end(), [](auto *InitExpr) { - return isa<DesignatedInitExpr>(InitExpr); - }); + const InitListExpr *SyntacticForm = + Node.isSyntacticForm() ? &Node : Node.getSyntacticForm(); + return std::all_of( + SyntacticForm->begin(), SyntacticForm->end(), + [](auto *InitExpr) { return isa<DesignatedInitExpr>(InitExpr); }); } AST_MATCHER(InitListExpr, hasSingleElement) { return Node.getNumInits() == 1; } 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 index 5eada72e225411..f65afa1def9a22 100644 --- 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 @@ -113,3 +113,18 @@ struct S10 { int i = 1, j = 2; }; S10 s101 {1, .j=2}; // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use designated init expression [modernize-use-designated-initializers] // CHECK-FIXES: S10 s101 {.i=1, .j=2}; + +struct S11 { int i; S10 s10; }; + +S11 s111 { .i = 1 }; +S11 s112 { 1 }; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] +// CHECK-FIXES: S11 s112 {.i=1}; + +S11 s113 { .i=1, {}}; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S11 s113 { .i=1, .s10={}}; + +S11 s114 { .i=1, .s10={1, .j=2}}; +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use designated init expression [modernize-use-designated-initializers] +// CHECK-FIXES: S11 s114 { .i=1, .s10={.i=1, .j=2}}; From f7f10129ae08560a228133c2303a792b45f63af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Tue, 13 Feb 2024 21:17:18 +0100 Subject: [PATCH 28/29] Have separate fixes --- .../modernize/UseDesignatedInitializersCheck.cpp | 16 +++++----------- .../modernize/use-designated-initializers.cpp | 4 ++-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp index d4cade4a604ae2..67327c306e2755 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -97,19 +97,13 @@ void UseDesignatedInitializersCheck::check( const llvm::DenseMap<clang::SourceLocation, std::string> Designators = clang::tooling::getUnwrittenDesignators(SyntacticInitList); if (isFullyUndesignated(SyntacticInitList)) { - std::string NewList = "{"; + DiagnosticBuilder Diag = + diag(InitList->getLBraceLoc(), "use designated initializer list"); for (const Stmt *InitExpr : *SyntacticInitList) { - if (InitExpr != *SyntacticInitList->begin()) - NewList += ", "; - NewList += Designators.at(InitExpr->getBeginLoc()); - NewList += "="; - NewList += Lexer::getSourceText( - CharSourceRange::getTokenRange(InitExpr->getSourceRange()), - *Result.SourceManager, getLangOpts()); + Diag << FixItHint::CreateInsertion( + InitExpr->getBeginLoc(), + Designators.at(InitExpr->getBeginLoc()) + "="); } - NewList += "}"; - diag(InitList->getLBraceLoc(), "use designated initializer list") - << FixItHint::CreateReplacement(InitList->getSourceRange(), NewList); } else { for (const auto *InitExpr : *SyntacticInitList) { if (!isa<DesignatedInitExpr>(InitExpr)) { 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 index f65afa1def9a22..a62ee1e609388e 100644 --- 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 @@ -52,7 +52,7 @@ S3 s32 = {{.i = 1, 2}}; // CHECK-MESSAGES: :[[@LINE-2]]:20: warning: use designated init expression [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list [modernize-use-designated-initializers] // CHECK-MESSAGES-POD: :[[@LINE-4]]:20: warning: use designated init expression [modernize-use-designated-initializers] -// CHECK-FIXES: S3 s32 = {.s2={.i = 1, 2}}; +// CHECK-FIXES: S3 s32 = {.s2={.i = 1, .j=2}}; struct S4 { double d; @@ -119,7 +119,7 @@ struct S11 { int i; S10 s10; }; S11 s111 { .i = 1 }; S11 s112 { 1 }; // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers] -// CHECK-FIXES: S11 s112 {.i=1}; +// CHECK-FIXES: S11 s112 { .i=1 }; S11 s113 { .i=1, {}}; // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use designated init expression [modernize-use-designated-initializers] From 6bba917af0361950a8c490a13d7ebf429a879f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Tue, 13 Feb 2024 21:41:13 +0100 Subject: [PATCH 29/29] Keep order in release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5d532e3cfb9a9c..11be24286d805d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,18 +100,18 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`modernize-use-designated-initializers + <clang-tidy/checks/modernize/use-designated-initializers>` check. + + Finds initializer lists for aggregate types that could be + written as designated initializers instead. + - New :doc:`readability-use-std-min-max <clang-tidy/checks/readability/use-std-min-max>` check. Replaces certain conditional statements with equivalent calls to ``std::min`` or ``std::max``. -- New :doc:`modernize-use-designated-initializers - <clang-tidy/checks/modernize/use-designated-initializers>` check. - - Finds initializer lists for aggregate type that could be - written as designated initializers instead. - New check aliases ^^^^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits