diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 28ca52f46943a..6852db6c2ee31 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -31,6 +31,7 @@ add_clang_library(clangTidyModernizeModule UseBoolLiteralsCheck.cpp UseConstraintsCheck.cpp UseDefaultMemberInitCheck.cpp + UseDesignatedInitializersCheck.cpp UseEmplaceCheck.cpp UseEqualsDefaultCheck.cpp UseEqualsDeleteCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 654f4bd0c6ba4..e96cf274f58cf 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -32,6 +32,7 @@ #include "UseBoolLiteralsCheck.h" #include "UseConstraintsCheck.h" #include "UseDefaultMemberInitCheck.h" +#include "UseDesignatedInitializersCheck.h" #include "UseEmplaceCheck.h" #include "UseEqualsDefaultCheck.h" #include "UseEqualsDeleteCheck.h" @@ -68,6 +69,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck("modernize-make-shared"); CheckFactories.registerCheck("modernize-make-unique"); CheckFactories.registerCheck("modernize-pass-by-value"); + CheckFactories.registerCheck( + "modernize-use-designated-initializers"); CheckFactories.registerCheck( "modernize-use-starts-ends-with"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp new file mode 100644 index 0000000000000..9ff6bb15043b5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp @@ -0,0 +1,184 @@ +//===--- 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 "../utils/DesignatedInitializers.h" +#include "clang/AST/APValue.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 "clang/ASTMatchers/ASTMatchersMacros.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static constexpr char IgnoreSingleElementAggregatesName[] = + "IgnoreSingleElementAggregates"; +static constexpr bool IgnoreSingleElementAggregatesDefault = true; + +static constexpr char RestrictToPODTypesName[] = "RestrictToPODTypes"; +static constexpr bool RestrictToPODTypesDefault = false; + +static constexpr char IgnoreMacrosName[] = "IgnoreMacros"; +static constexpr bool IgnoreMacrosDefault = true; + +namespace { + +struct Designators { + + Designators(const InitListExpr *InitList) : InitList(InitList) { + assert(InitList->isSyntacticForm()); + }; + + unsigned size() { return getCached().size(); } + + std::optional operator[](const SourceLocation &Location) { + const auto Designators = getCached(); + const auto Result = Designators.find(Location); + if (Result == Designators.end()) + return {}; + const llvm::StringRef Designator = Result->getSecond(); + return (Designator.front() == '.' ? Designator.substr(1) : Designator) + .trim("\0"); // Trim NULL characters appearing on Windows in the + // name. + } + +private: + using LocationToNameMap = llvm::DenseMap; + + std::optional CachedDesignators; + const InitListExpr *InitList; + + LocationToNameMap &getCached() { + return CachedDesignators ? *CachedDesignators + : CachedDesignators.emplace( + utils::getUnwrittenDesignators(InitList)); + } +}; + +unsigned getNumberOfDesignated(const InitListExpr *SyntacticInitList) { + return llvm::count_if(*SyntacticInitList, [](auto *InitExpr) { + return isa(InitExpr); + }); +} + +AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); } + +AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); } + +AST_MATCHER(InitListExpr, isFullyDesignated) { + if (const InitListExpr *SyntacticForm = + Node.isSyntacticForm() ? &Node : Node.getSyntacticForm()) + return getNumberOfDesignated(SyntacticForm) == SyntacticForm->getNumInits(); + return true; +} + +AST_MATCHER(InitListExpr, hasMoreThanOneElement) { + return Node.getNumInits() > 1; +} + +} // namespace + +UseDesignatedInitializersCheck::UseDesignatedInitializersCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), IgnoreSingleElementAggregates(Options.get( + IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregatesDefault)), + RestrictToPODTypes( + Options.get(RestrictToPODTypesName, RestrictToPODTypesDefault)), + IgnoreMacros( + Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)) {} + +void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) { + const auto HasBaseWithFields = + hasAnyBase(hasType(cxxRecordDecl(has(fieldDecl())))); + Finder->addMatcher( + initListExpr( + hasType(cxxRecordDecl(RestrictToPODTypes ? isPOD() : isAggregate(), + unless(HasBaseWithFields)) + .bind("type")), + IgnoreSingleElementAggregates ? hasMoreThanOneElement() : anything(), + unless(isFullyDesignated())) + .bind("init"), + this); +} + +void UseDesignatedInitializersCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *InitList = Result.Nodes.getNodeAs("init"); + const auto *Type = Result.Nodes.getNodeAs("type"); + if (!Type || !InitList) + return; + const auto *SyntacticInitList = InitList->getSyntacticForm(); + if (!SyntacticInitList) + return; + Designators Designators{SyntacticInitList}; + const unsigned NumberOfDesignated = getNumberOfDesignated(SyntacticInitList); + if (SyntacticInitList->getNumInits() - NumberOfDesignated > + Designators.size()) + return; + + // If the whole initializer list is un-designated, issue only one warning and + // a single fix-it for the whole expression. + if (0 == NumberOfDesignated) { + if (IgnoreMacros && InitList->getBeginLoc().isMacroID()) + return; + { + DiagnosticBuilder Diag = + diag(InitList->getLBraceLoc(), + "use designated initializer list to initialize %0"); + Diag << Type << InitList->getSourceRange(); + for (const Stmt *InitExpr : *SyntacticInitList) { + const auto Designator = Designators[InitExpr->getBeginLoc()]; + if (Designator && !Designator->empty()) + Diag << FixItHint::CreateInsertion(InitExpr->getBeginLoc(), + ("." + *Designator + "=").str()); + } + } + diag(Type->getBeginLoc(), "aggregate type is defined here", + DiagnosticIDs::Note); + return; + } + + // In case that only a few elements are un-designated (not all as before), the + // check offers dedicated issues and fix-its for each of them. + for (const auto *InitExpr : *SyntacticInitList) { + if (isa(InitExpr)) + continue; + if (IgnoreMacros && InitExpr->getBeginLoc().isMacroID()) + continue; + const auto Designator = Designators[InitExpr->getBeginLoc()]; + if (!Designator || Designator->empty()) { + // There should always be a designator. If there's unexpectedly none, we + // at least report a generic diagnostic. + diag(InitExpr->getBeginLoc(), "use designated init expression") + << InitExpr->getSourceRange(); + } else { + diag(InitExpr->getBeginLoc(), + "use designated init expression to initialize field '%0'") + << InitExpr->getSourceRange() << *Designator + << FixItHint::CreateInsertion(InitExpr->getBeginLoc(), + ("." + *Designator + "=").str()); + } + } +} + +void UseDesignatedInitializersCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, IgnoreSingleElementAggregatesName, + IgnoreSingleElementAggregates); + Options.store(Opts, RestrictToPODTypesName, RestrictToPODTypes); + Options.store(Opts, IgnoreMacrosName, IgnoreMacros); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h new file mode 100644 index 0000000000000..0a496f51b9576 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.h @@ -0,0 +1,40 @@ +//===--- 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 { + +/// Finds initializer lists for aggregate type that could be +/// written as designated initializers instead. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-designated-initializers.html +class UseDesignatedInitializersCheck : public ClangTidyCheck { +public: + UseDesignatedInitializersCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + bool IgnoreSingleElementAggregates; + bool RestrictToPODTypes; + bool IgnoreMacros; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEDESIGNATEDINITIALIZERSCHECK_H diff --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt index 88638d4acd556..f0160fa9df748 100644 --- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -7,6 +7,7 @@ add_clang_library(clangTidyUtils Aliasing.cpp ASTUtils.cpp DeclRefExprUtils.cpp + DesignatedInitializers.cpp ExceptionAnalyzer.cpp ExceptionSpecAnalyzer.cpp ExprSequence.cpp diff --git a/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.cpp b/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.cpp new file mode 100644 index 0000000000000..6faeb7a0b76e1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.cpp @@ -0,0 +1,195 @@ +//===--- DesignatedInitializers.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 +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides utilities for designated initializers. +/// +//===----------------------------------------------------------------------===// + +#include "DesignatedInitializers.h" +#include "clang/AST/DeclCXX.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/ScopeExit.h" + +namespace clang::tidy::utils { + +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(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 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 &Out, + const llvm::DenseSet &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(Init)) + continue; + + const auto *BraceElidedSubobject = llvm::dyn_cast(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 +getUnwrittenDesignators(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 NestedBraces; + for (const Expr *Init : Syn->inits()) + if (auto *Nested = llvm::dyn_cast(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 Designators; + std::string EmptyPrefix; + collectDesignators(Syn->isSemanticForm() ? Syn : Syn->getSemanticForm(), + Designators, NestedBraces, EmptyPrefix); + return Designators; +} + +} // namespace clang::tidy::utils diff --git a/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.h b/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.h new file mode 100644 index 0000000000000..a6cb2963faf72 --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/DesignatedInitializers.h @@ -0,0 +1,42 @@ +//===--- DesignatedInitializers.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 +// +//===----------------------------------------------------------------------===// +/// +/// \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::tidy::utils { + +/// Get designators describing the elements of a (syntactic) init list. +/// +/// Given for example the type +/// \code +/// struct S { int i, j; }; +/// \endcode +/// and the definition +/// \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. +llvm::DenseMap +getUnwrittenDesignators(const clang::InitListExpr *Syn); + +} // namespace clang::tidy::utils diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index 3911fb6c6c746..f49704157880d 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -182,6 +182,7 @@ target_link_libraries(clangDaemon clangIncludeCleaner clangPseudo clangTidy + clangTidyUtils clangdSupport ) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 671a9c30ffa95..a0ebc631ef828 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// #include "InlayHints.h" +#include "../clang-tidy/utils/DesignatedInitializers.h" #include "AST.h" #include "Config.h" #include "HeuristicResolver.h" @@ -24,7 +25,6 @@ #include "clang/Basic/OperatorKinds.h" #include "clang/Basic/SourceManager.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(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 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 &Out, - const llvm::DenseSet &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(Init)) - continue; - - const auto *BraceElidedSubobject = llvm::dyn_cast(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 -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 NestedBraces; - for (const Expr *Init : Syn->inits()) - if (auto *Nested = llvm::dyn_cast(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 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. @@ -847,14 +684,15 @@ class InlayHintVisitor : public RecursiveASTVisitor { // 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 Designators = - getDesignators(Syn); + tidy::utils::getUnwrittenDesignators(Syn); for (const Expr *Init : Syn->inits()) { if (llvm::isa(Init)) continue; diff --git a/clang-tools-extra/clangd/tool/CMakeLists.txt b/clang-tools-extra/clangd/tool/CMakeLists.txt index 6c21175d7687c..4012b6401c008 100644 --- a/clang-tools-extra/clangd/tool/CMakeLists.txt +++ b/clang-tools-extra/clangd/tool/CMakeLists.txt @@ -33,6 +33,7 @@ clang_target_link_libraries(clangdMain target_link_libraries(clangdMain PRIVATE clangTidy + clangTidyUtils clangDaemon clangdRemoteIndex diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 9cd195eaf164f..e432db8d0912e 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -175,6 +175,7 @@ target_link_libraries(ClangdTests clangIncludeCleaner clangTesting clangTidy + clangTidyUtils clangdSupport ) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3f90e7d63d6b2..5bae530e94238 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`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 ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 59ef69f390ee9..5e57bc0ee483f 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 `, "Yes" :doc:`modernize-use-constraints `, "Yes" :doc:`modernize-use-default-member-init `, "Yes" + :doc:`modernize-use-designated-initializers `, "Yes" :doc:`modernize-use-emplace `, "Yes" :doc:`modernize-use-equals-default `, "Yes" :doc:`modernize-use-equals-delete `, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst new file mode 100644 index 0000000000000..22f50980baade --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-designated-initializers.rst @@ -0,0 +1,62 @@ +.. title:: clang-tidy - modernize-use-designated-initializers + +modernize-use-designated-initializers +===================================== + +Finds initializer lists for aggregate types which 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 every variable being initialized is referenced by its 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``. + +Even when compiling in a language version older than C++20, depending on your +compiler, designated initializers are potentially supported. Therefore, the +check is not restricted to C++20 and newer versions. Check out the options +``-Wc99-designator`` to get support for mixed designators in initializer list in +C and ``-Wc++20-designator`` for support of designated initializers in older C++ +language modes. + +Options +------- + +.. option:: IgnoreMacros + + The value `false` specifies that components of initializer lists expanded from + macros are not checked. The default value is `true`. + +.. 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`. + +.. 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 new file mode 100644 index 0000000000000..7e5c26e3f4404 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp @@ -0,0 +1,203 @@ +// RUN: %check_clang_tidy -std=c++17 %s modernize-use-designated-initializers %t \ +// RUN: -- \ +// RUN: -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=,SINGLE-ELEMENT -std=c++17 %s modernize-use-designated-initializers %t \ +// RUN: -- -config="{CheckOptions: {modernize-use-designated-initializers.IgnoreSingleElementAggregates: false}}" \ +// RUN: -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=POD -std=c++17 %s modernize-use-designated-initializers %t \ +// RUN: -- -config="{CheckOptions: {modernize-use-designated-initializers.RestrictToPODTypes: true}}" \ +// RUN: -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=,MACROS -std=c++17 %s modernize-use-designated-initializers %t \ +// RUN: -- -config="{CheckOptions: {modernize-use-designated-initializers.IgnoreMacros: false}}" \ +// RUN: -- -fno-delayed-template-parsing + +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 to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-6]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-8]]:1: note: aggregate type is defined here +// CHECK-FIXES: S2 s22 = {.i=1, .j=2}; + +S2 s23{1}; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-13]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-3]]:7: warning: use designated initializer list to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-15]]:1: note: aggregate type is defined here +// 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 to initialize field 'j' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-2]]:17: warning: use designated init expression to initialize field 'j' [modernize-use-designated-initializers] +// CHECK-FIXES: S2 s25 = {.i=1, .j=2}; + +class S3 { + public: + S2 s2; + double d; +}; + +S3 s31 = {.s2 = 1, 2, 3.1}; +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use designated init expression to initialize field 's2.j' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: use designated init expression to initialize field 'd' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-3]]:20: warning: use designated init expression to initialize field 's2.j' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-4]]:23: warning: use designated init expression to initialize field 'd' [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 to initialize 'S3' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-15]]:1: note: aggregate type is defined here +// CHECK-MESSAGES: :[[@LINE-3]]:20: warning: use designated init expression to initialize field 'j' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-4]]:10: warning: use designated initializer list to initialize 'S3' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-18]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-6]]:20: warning: use designated init expression to initialize field 'j' [modernize-use-designated-initializers] +// CHECK-FIXES: S3 s32 = {.s2={.i = 1, .j=2}}; + +S3 s33 = {{2}, .d=3.1}; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use designated init expression to initialize field 's2' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: use designated initializer list to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-50]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-4]]:11: warning: use designated init expression to initialize field 's2' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-5]]:11: warning: use designated initializer list to initialize 'S2' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-53]]:1: note: aggregate type is defined here +// CHECK-FIXES: S3 s33 = {.s2={.i=2}, .d=3.1}; + +struct S4 { + double d; + private: static int i; +}; + +S4 s41 {2.2}; +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list to initialize 'S4' [modernize-use-designated-initializers] +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-7]]:1: note: aggregate type is defined here +// CHECK-FIXES-SINGLE-ELEMENT: S4 s41 {.d=2.2}; + +S4 s42 = {{}}; +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:10: warning: use designated initializer list to initialize 'S4' [modernize-use-designated-initializers] +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-12]]:1: note: aggregate type is defined here +// CHECK-FIXES-SINGLE-ELEMENT: S4 s42 = {.d={}}; + +template S template1() { return {10, 11}; } + +S2 s26 = template1(); + +template S template2() { return {}; } + +S2 s27 = template2(); + +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}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use designated initializer list to initialize 'S6' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-7]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-3]]:8: warning: use designated initializer list to initialize 'S6' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-9]]:1: note: aggregate type is defined here +// CHECK-FIXES: S6 s61 {.i=1, .s.j=2}; + +struct S7 { + union { + int k; + double d; + } u; +}; + +S7 s71 {1}; +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list to initialize 'S7' [modernize-use-designated-initializers] +// CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-9]]:1: note: aggregate type is defined here +// CHECK-FIXES-SINGLE-ELEMENT: S7 s71 {.u.k=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 to initialize 'S9' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-7]]:1: note: aggregate type is defined here +// 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 to initialize field 'i' [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 to initialize 'S11' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-5]]:1: note: aggregate type is defined here +// CHECK-FIXES: S11 s112 { .i=1 }; + +S11 s113 { .i=1, {}}; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use designated init expression to initialize field 's10' [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 to initialize field 'i' [modernize-use-designated-initializers] +// CHECK-FIXES: S11 s114 { .i=1, .s10={.i=1, .j=2}}; + +struct S12 { + int i; + struct { int j; }; +}; + +S12 s121 {1, 2}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list to initialize 'S12' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-7]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list to initialize 'S12' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-9]]:1: note: aggregate type is defined here +// CHECK-FIXES: S12 s121 {.i=1, .j=2}; + +struct S13 { + union { + int k; + double d; + }; + int i; +}; + +S13 s131 {1, 2}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list to initialize 'S13' [modernize-use-designated-initializers] +// CHECK-MESSAGES: :[[@LINE-10]]:1: note: aggregate type is defined here +// CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list to initialize 'S13' [modernize-use-designated-initializers] +// CHECK-MESSAGES-POD: :[[@LINE-12]]:1: note: aggregate type is defined here +// CHECK-FIXES: S13 s131 {.k=1, .i=2}; + +#define A (3+2) +#define B .j=1 + +S9 s92 {A, B}; +// CHECK-MESSAGES-MACROS: :[[@LINE-1]]:9: warning: use designated init expression to initialize field 'i' [modernize-use-designated-initializers] +// CHECK-MESSAGES-MACROS: :[[@LINE-5]]:11: note: expanded from macro 'A' + +#define DECLARE_S93 S9 s93 {1, 2} + +DECLARE_S93; +// CHECK-MESSAGES-MACROS: :[[@LINE-1]]:1: warning: use designated initializer list to initialize 'S9' [modernize-use-designated-initializers] +// CHECK-MESSAGES-MACROS: :[[@LINE-4]]:28: note: expanded from macro 'DECLARE_S93' +// CHECK-MESSAGES-MACROS: :[[@LINE-71]]:1: note: aggregate type is defined here