Author: Vladimir Vuksanovic Date: 2025-09-01T00:26:42-07:00 New Revision: 4d9578b8ed20f293641a5917447885ab97aeefbe
URL: https://github.com/llvm/llvm-project/commit/4d9578b8ed20f293641a5917447885ab97aeefbe DIFF: https://github.com/llvm/llvm-project/commit/4d9578b8ed20f293641a5917447885ab97aeefbe.diff LOG: [clang-reorder-fields] Support designated initializers (#142150) Initializer lists with designators, missing elements or omitted braces can now be rewritten. Any missing designators are added and they get sorted according to the new order. ``` struct Foo { int a; int b; int c; }; struct Foo foo = { .a = 1, 2, 3 } ``` when reordering elements to "b,a,c" becomes: ``` struct Foo { int b; int a; int c; }; struct Foo foo = { .b = 2, .a = 1, .c = 3 } ``` Added: clang-tools-extra/clang-reorder-fields/Designator.cpp clang-tools-extra/clang-reorder-fields/Designator.h clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.c clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.cpp clang-tools-extra/test/clang-reorder-fields/IdiomaticZeroInitializer.c clang-tools-extra/test/clang-reorder-fields/InitializerListExcessElements.c Modified: clang-tools-extra/clang-reorder-fields/CMakeLists.txt clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt index 2fdeb65d89767..dec0287b26873 100644 --- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt +++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangReorderFields STATIC + Designator.cpp ReorderFieldsAction.cpp DEPENDS diff --git a/clang-tools-extra/clang-reorder-fields/Designator.cpp b/clang-tools-extra/clang-reorder-fields/Designator.cpp new file mode 100644 index 0000000000000..fc070f7f6746d --- /dev/null +++ b/clang-tools-extra/clang-reorder-fields/Designator.cpp @@ -0,0 +1,219 @@ +//===-- tools/extra/clang-reorder-fields/utils/Designator.cpp ---*- 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 contains the definition of the Designator and Designators utility +/// classes. +/// +//===----------------------------------------------------------------------===// + +#include "Designator.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "llvm/Support/raw_ostream.h" + +namespace clang { +namespace reorder_fields { + +void Designator::advanceToNextField() { + assert(!isFinished() && "Iterator is already finished"); + switch (Tag) { + case STRUCT: + if (StructIt.Record->isUnion()) { + // Union always finishes on first increment. + StructIt.Field = StructIt.Record->field_end(); + Type = QualType(); + break; + } + ++StructIt.Field; + if (StructIt.Field != StructIt.Record->field_end()) { + Type = StructIt.Field->getType(); + } else { + Type = QualType(); + } + break; + case ARRAY: + ++ArrayIt.Index; + break; + case ARRAY_RANGE: + ArrayIt.Index = ArrayRangeIt.End + 1; + ArrayIt.Size = ArrayRangeIt.Size; + Tag = ARRAY; + break; + } +} + +bool Designator::isFinished() { + switch (Tag) { + case STRUCT: + return StructIt.Field == StructIt.Record->field_end(); + case ARRAY: + return ArrayIt.Index == ArrayIt.Size; + case ARRAY_RANGE: + return ArrayRangeIt.End == ArrayRangeIt.Size; + } + return false; +} + +Designators::Designators(const Expr *Init, const InitListExpr *ILE, + const ASTContext *Context) + : ILE(ILE), Context(Context) { + if (ILE->getType()->isArrayType()) { + const ConstantArrayType *CAT = + Context->getAsConstantArrayType(ILE->getType()); + // Only constant size arrays are supported. + if (!CAT) { + DesignatorList.clear(); + return; + } + DesignatorList.push_back( + {CAT->getElementType(), 0, CAT->getSize().getZExtValue()}); + } else { + const RecordDecl *DesignatorRD = ILE->getType()->getAsRecordDecl(); + DesignatorList.push_back({DesignatorRD->field_begin()->getType(), + DesignatorRD->field_begin(), DesignatorRD}); + } + + // If the designator list is empty at this point, then there must be excess + // elements in the initializer list. They are not currently supported. + if (DesignatorList.empty()) + return; + + if (!enterImplicitInitLists(Init)) + DesignatorList.clear(); +} + +Designators::Designators(const DesignatedInitExpr *DIE, const InitListExpr *ILE, + const ASTContext *Context) + : ILE(ILE), Context(Context) { + for (const auto &D : DIE->designators()) { + if (D.isFieldDesignator()) { + RecordDecl *DesignatorRecord = D.getFieldDecl()->getParent(); + for (auto FieldIt = DesignatorRecord->field_begin(); + FieldIt != DesignatorRecord->field_end(); ++FieldIt) { + if (*FieldIt == D.getFieldDecl()) { + DesignatorList.push_back( + {FieldIt->getType(), FieldIt, DesignatorRecord}); + break; + } + } + } else { + const QualType CurrentType = DesignatorList.empty() + ? ILE->getType() + : DesignatorList.back().getType(); + const ConstantArrayType *CAT = + Context->getAsConstantArrayType(CurrentType); + if (!CAT) { + // Non-constant-sized arrays are not supported. + DesignatorList.clear(); + return; + } + if (D.isArrayDesignator()) { + DesignatorList.push_back({CAT->getElementType(), + DIE->getArrayIndex(D) + ->EvaluateKnownConstInt(*Context) + .getZExtValue(), + CAT->getSize().getZExtValue()}); + } else if (D.isArrayRangeDesignator()) { + DesignatorList.push_back({CAT->getElementType(), + DIE->getArrayRangeStart(D) + ->EvaluateKnownConstInt(*Context) + .getZExtValue(), + DIE->getArrayRangeEnd(D) + ->EvaluateKnownConstInt(*Context) + .getZExtValue(), + CAT->getSize().getZExtValue()}); + } else { + llvm_unreachable("Unexpected designator kind"); + } + } + } +} + +bool Designators::advanceToNextField(const Expr *Init) { + // Remove all designators that refer to the last field of a struct or final + // element of the array. + while (!DesignatorList.empty()) { + auto &CurrentDesignator = DesignatorList.back(); + CurrentDesignator.advanceToNextField(); + if (CurrentDesignator.isFinished()) { + DesignatorList.pop_back(); + continue; + } + break; + } + + // If the designator list is empty at this point, then there must be excess + // elements in the initializer list. They are not currently supported. + if (DesignatorList.empty()) + return false; + + if (!enterImplicitInitLists(Init)) { + DesignatorList.clear(); + return false; + } + + return true; +} + +bool Designators::enterImplicitInitLists(const Expr *Init) { + // Check for missing braces by comparing the type of the last designator and + // type of Init. + while (true) { + const QualType T = DesignatorList.back().getType(); + // If the types match, there are no missing braces. + if (Init->getType() == T) + break; + + // If the current type is a struct, then get its first field. + if (T->isRecordType()) { + DesignatorList.push_back({T->getAsRecordDecl()->field_begin()->getType(), + T->getAsRecordDecl()->field_begin(), + T->getAsRecordDecl()}); + continue; + } + // If the current type is an array, then get its first element. + if (T->isArrayType()) { + DesignatorList.push_back( + {Context->getAsArrayType(T)->getElementType(), 0, + Context->getAsConstantArrayType(T)->getSize().getZExtValue()}); + continue; + } + + // The initializer doesn't match the expected type. The initializer list is + // invalid. + return false; + } + + return true; +} + +std::string Designators::toString() const { + if (DesignatorList.empty()) + return ""; + std::string Designator; + llvm::raw_string_ostream OS(Designator); + for (auto &I : DesignatorList) { + switch (I.getTag()) { + case Designator::STRUCT: + OS << '.' << I.getStructIter()->getName(); + break; + case Designator::ARRAY: + OS << '[' << I.getArrayIndex() << ']'; + break; + case Designator::ARRAY_RANGE: + OS << '[' << I.getArrayRangeStart() << "..." << I.getArrayRangeEnd() + << ']'; + } + } + OS << " = "; + return Designator; +} + +} // namespace reorder_fields +} // namespace clang diff --git a/clang-tools-extra/clang-reorder-fields/Designator.h b/clang-tools-extra/clang-reorder-fields/Designator.h new file mode 100644 index 0000000000000..09c0a6ce27e26 --- /dev/null +++ b/clang-tools-extra/clang-reorder-fields/Designator.h @@ -0,0 +1,165 @@ +//===-- tools/extra/clang-reorder-fields/utils/Designator.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 contains the declarations of the Designator and Designators +/// utility classes. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_UTILS_DESIGNATOR_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_UTILS_DESIGNATOR_H + +#include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/Type.h" + +namespace clang { +namespace reorder_fields { + +/// Represents a part of a designation in a C99/C++20 designated initializer. It +/// is a tagged union of diff erent kinds of designators: struct, array and array +/// range. Holds enough information to be able to advance to the next field and +/// to know when all fields have been iterated through. +class Designator { +public: + enum Kind { STRUCT, ARRAY, ARRAY_RANGE }; + + Designator(const QualType Type, RecordDecl::field_iterator Field, + const RecordDecl *RD) + : Tag(STRUCT), Type(Type), StructIt({Field, RD}) {} + + Designator(const QualType Type, uint64_t Idx, uint64_t Size) + : Tag(ARRAY), Type(Type), ArrayIt({Idx, Size}) {} + + Designator(const QualType Type, uint64_t Start, uint64_t End, uint64_t Size) + : Tag(ARRAY_RANGE), Type(Type), ArrayRangeIt({Start, End, Size}) {} + + /// Moves the iterator to the next element. + void advanceToNextField(); + + /// Checks if the iterator has iterated through all elements. + bool isFinished(); + + Kind getTag() const { return Tag; } + QualType getType() const { return Type; } + + const RecordDecl::field_iterator getStructIter() const { + assert(Tag == STRUCT && "Must be a field designator"); + return StructIt.Field; + } + + const RecordDecl *getStructDecl() const { + assert(Tag == STRUCT && "Must be a field designator"); + return StructIt.Record; + } + + uint64_t getArrayIndex() const { + assert(Tag == ARRAY && "Must be an array designator"); + return ArrayIt.Index; + } + + uint64_t getArrayRangeStart() const { + assert(Tag == ARRAY_RANGE && "Must be an array range designator"); + return ArrayRangeIt.Start; + } + + uint64_t getArrayRangeEnd() const { + assert(Tag == ARRAY_RANGE && "Must be an array range designator"); + return ArrayRangeIt.End; + } + + uint64_t getArraySize() const { + assert((Tag == ARRAY || Tag == ARRAY_RANGE) && + "Must be an array or range designator"); + if (Tag == ARRAY) + return ArrayIt.Size; + return ArrayRangeIt.Size; + } + +private: + /// Type of the designator. + Kind Tag; + + /// Type of the designated entry. For arrays this is the type of the element. + QualType Type; + + /// Field designator has the iterator to the field and the record the field + /// is declared in. + struct StructIter { + RecordDecl::field_iterator Field; + const RecordDecl *Record; + }; + + /// Array designator has an index and size of the array. + struct ArrayIter { + uint64_t Index; + uint64_t Size; + }; + + /// Array range designator has a start and end index and size of the array. + struct ArrayRangeIter { + uint64_t Start; + uint64_t End; + uint64_t Size; + }; + + union { + StructIter StructIt; + ArrayIter ArrayIt; + ArrayRangeIter ArrayRangeIt; + }; +}; + +/// List of designators. +class Designators { +public: + /// Initialize to the first member of the struct/array. Enters implicit + /// initializer lists until a type that matches Init is found. + Designators(const Expr *Init, const InitListExpr *ILE, + const ASTContext *Context); + + /// Initialize to the designators of the given expression. + Designators(const DesignatedInitExpr *DIE, const InitListExpr *ILE, + const ASTContext *Context); + + /// Return whether this designator list is valid. + bool isValid() const { return !DesignatorList.empty(); } + + /// Moves the designators to the next initializer in the struct/array. If the + /// type of next initializer doesn't match the expected type then there are + /// omitted braces and we add new designators to reflect that. + bool advanceToNextField(const Expr *Init); + + /// Gets a string representation from a list of designators. This string will + /// be inserted before an initializer expression to make it designated. + std::string toString() const; + + size_t size() const { return DesignatorList.size(); } + + SmallVector<Designator>::const_iterator begin() const { + return DesignatorList.begin(); + } + SmallVector<Designator>::const_iterator end() const { + return DesignatorList.end(); + } + +private: + /// Enters any implicit initializer lists until a type that matches the given + /// expression is found. + bool enterImplicitInitLists(const Expr *Init); + + const InitListExpr *ILE; + const ASTContext *Context; + SmallVector<Designator, 1> DesignatorList; +}; + +} // namespace reorder_fields +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_UTILS_DESIGNATOR_H diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp index ada9122b587ae..affa276a0c550 100644 --- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp +++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "ReorderFieldsAction.h" +#include "Designator.h" #include "clang/AST/AST.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" @@ -25,6 +26,7 @@ #include "clang/Tooling/Refactoring.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SetVector.h" +#include "llvm/Support/ErrorHandling.h" #include <string> namespace clang { @@ -162,7 +164,91 @@ getNewFieldsOrder(const RecordDecl *Definition, return NewFieldsOrder; } +struct ReorderedStruct { +public: + ReorderedStruct(const RecordDecl *Decl, ArrayRef<unsigned> NewFieldsOrder) + : Definition(Decl), NewFieldsOrder(NewFieldsOrder), + NewFieldsPositions(NewFieldsOrder.size()) { + for (unsigned I = 0; I < NewFieldsPositions.size(); ++I) + NewFieldsPositions[NewFieldsOrder[I]] = I; + } + + /// Compares compatible designators according to the new struct order. + /// Returns a negative value if Lhs < Rhs, positive value if Lhs > Rhs and 0 + /// if they are equal. + bool operator()(const Designator &Lhs, const Designator &Rhs) const; + + /// Compares compatible designator lists according to the new struct order. + /// Returns a negative value if Lhs < Rhs, positive value if Lhs > Rhs and 0 + /// if they are equal. + bool operator()(const Designators &Lhs, const Designators &Rhs) const; + + const RecordDecl *Definition; + ArrayRef<unsigned> NewFieldsOrder; + SmallVector<unsigned, 4> NewFieldsPositions; +}; + +bool ReorderedStruct::operator()(const Designator &Lhs, + const Designator &Rhs) const { + switch (Lhs.getTag()) { + case Designator::STRUCT: + assert(Rhs.getTag() == Designator::STRUCT && "Incompatible designators"); + assert(Lhs.getStructDecl() == Rhs.getStructDecl() && + "Incompatible structs"); + // Use the new layout for reordered struct. + if (Definition == Lhs.getStructDecl()) { + return NewFieldsPositions[Lhs.getStructIter()->getFieldIndex()] < + NewFieldsPositions[Rhs.getStructIter()->getFieldIndex()]; + } + return Lhs.getStructIter()->getFieldIndex() < + Rhs.getStructIter()->getFieldIndex(); + case Designator::ARRAY: + case Designator::ARRAY_RANGE: + // Array designators can be compared to array range designators. + assert((Rhs.getTag() == Designator::ARRAY || + Rhs.getTag() == Designator::ARRAY_RANGE) && + "Incompatible designators"); + size_t LhsIdx = Lhs.getTag() == Designator::ARRAY + ? Lhs.getArrayIndex() + : Lhs.getArrayRangeStart(); + size_t RhsIdx = Rhs.getTag() == Designator::ARRAY + ? Rhs.getArrayIndex() + : Rhs.getArrayRangeStart(); + return LhsIdx < RhsIdx; + } + llvm_unreachable("Invalid designator tag"); +} + +bool ReorderedStruct::operator()(const Designators &Lhs, + const Designators &Rhs) const { + return std::lexicographical_compare(Lhs.begin(), Lhs.end(), Rhs.begin(), + Rhs.end(), *this); +} + // FIXME: error-handling +/// Replaces a range of source code by the specified text. +static void +addReplacement(SourceRange Old, StringRef New, const ASTContext &Context, + std::map<std::string, tooling::Replacements> &Replacements) { + tooling::Replacement R(Context.getSourceManager(), + CharSourceRange::getTokenRange(Old), New, + Context.getLangOpts()); + consumeError(Replacements[std::string(R.getFilePath())].add(R)); +} + +/// Replaces one range of source code by another and adds a prefix. +static void +addReplacement(SourceRange Old, SourceRange New, StringRef Prefix, + const ASTContext &Context, + std::map<std::string, tooling::Replacements> &Replacements) { + std::string NewText = + (Prefix + Lexer::getSourceText(CharSourceRange::getTokenRange(New), + Context.getSourceManager(), + Context.getLangOpts())) + .str(); + addReplacement(Old, NewText, Context, Replacements); +} + /// Replaces one range of source code by another. static void addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context, @@ -174,10 +260,7 @@ addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context, StringRef NewText = Lexer::getSourceText(CharSourceRange::getTokenRange(New), Context.getSourceManager(), Context.getLangOpts()); - tooling::Replacement R(Context.getSourceManager(), - CharSourceRange::getTokenRange(Old), NewText, - Context.getLangOpts()); - consumeError(Replacements[std::string(R.getFilePath())].add(R)); + addReplacement(Old, NewText.str(), Context, Replacements); } /// Find all member fields used in the given init-list initializer expr @@ -279,33 +362,33 @@ static SourceRange getFullFieldSourceRange(const FieldDecl &Field, /// diff erent accesses (public/protected/private) is not supported. /// \returns true on success. static bool reorderFieldsInDefinition( - const RecordDecl *Definition, ArrayRef<unsigned> NewFieldsOrder, - const ASTContext &Context, + const ReorderedStruct &RS, const ASTContext &Context, std::map<std::string, tooling::Replacements> &Replacements) { - assert(Definition && "Definition is null"); + assert(RS.Definition && "Definition is null"); SmallVector<const FieldDecl *, 10> Fields; - for (const auto *Field : Definition->fields()) + for (const auto *Field : RS.Definition->fields()) Fields.push_back(Field); // Check that the permutation of the fields doesn't change the accesses - for (const auto *Field : Definition->fields()) { + for (const auto *Field : RS.Definition->fields()) { const auto FieldIndex = Field->getFieldIndex(); - if (Field->getAccess() != Fields[NewFieldsOrder[FieldIndex]]->getAccess()) { + if (Field->getAccess() != + Fields[RS.NewFieldsOrder[FieldIndex]]->getAccess()) { llvm::errs() << "Currently reordering of fields with diff erent accesses " "is not supported\n"; return false; } } - for (const auto *Field : Definition->fields()) { + for (const auto *Field : RS.Definition->fields()) { const auto FieldIndex = Field->getFieldIndex(); - if (FieldIndex == NewFieldsOrder[FieldIndex]) + if (FieldIndex == RS.NewFieldsOrder[FieldIndex]) continue; - addReplacement( - getFullFieldSourceRange(*Field, Context), - getFullFieldSourceRange(*Fields[NewFieldsOrder[FieldIndex]], Context), - Context, Replacements); + addReplacement(getFullFieldSourceRange(*Field, Context), + getFullFieldSourceRange( + *Fields[RS.NewFieldsOrder[FieldIndex]], Context), + Context, Replacements); } return true; } @@ -316,7 +399,7 @@ static bool reorderFieldsInDefinition( /// fields. Thus, we need to ensure that we reorder just the initializers that /// are present. static void reorderFieldsInConstructor( - const CXXConstructorDecl *CtorDecl, ArrayRef<unsigned> NewFieldsOrder, + const CXXConstructorDecl *CtorDecl, const ReorderedStruct &RS, ASTContext &Context, std::map<std::string, tooling::Replacements> &Replacements) { assert(CtorDecl && "Constructor declaration is null"); @@ -328,10 +411,6 @@ static void reorderFieldsInConstructor( // Thus this assert needs to be after the previous checks. assert(CtorDecl->isThisDeclarationADefinition() && "Not a definition"); - SmallVector<unsigned, 10> NewFieldsPositions(NewFieldsOrder.size()); - for (unsigned i = 0, e = NewFieldsOrder.size(); i < e; ++i) - NewFieldsPositions[NewFieldsOrder[i]] = i; - SmallVector<const CXXCtorInitializer *, 10> OldWrittenInitializersOrder; SmallVector<const CXXCtorInitializer *, 10> NewWrittenInitializersOrder; for (const auto *Initializer : CtorDecl->inits()) { @@ -342,8 +421,8 @@ static void reorderFieldsInConstructor( const FieldDecl *ThisM = Initializer->getMember(); const auto UsedMembers = findMembersUsedInInitExpr(Initializer, Context); for (const FieldDecl *UM : UsedMembers) { - if (NewFieldsPositions[UM->getFieldIndex()] > - NewFieldsPositions[ThisM->getFieldIndex()]) { + if (RS.NewFieldsPositions[UM->getFieldIndex()] > + RS.NewFieldsPositions[ThisM->getFieldIndex()]) { DiagnosticsEngine &DiagEngine = Context.getDiagnostics(); auto Description = ("reordering field " + UM->getName() + " after " + ThisM->getName() + " makes " + UM->getName() + @@ -361,8 +440,8 @@ static void reorderFieldsInConstructor( auto ByFieldNewPosition = [&](const CXXCtorInitializer *LHS, const CXXCtorInitializer *RHS) { assert(LHS && RHS); - return NewFieldsPositions[LHS->getMember()->getFieldIndex()] < - NewFieldsPositions[RHS->getMember()->getFieldIndex()]; + return RS.NewFieldsPositions[LHS->getMember()->getFieldIndex()] < + RS.NewFieldsPositions[RHS->getMember()->getFieldIndex()]; }; llvm::sort(NewWrittenInitializersOrder, ByFieldNewPosition); assert(OldWrittenInitializersOrder.size() == @@ -374,35 +453,188 @@ static void reorderFieldsInConstructor( Replacements); } +/// Replacement for broken InitListExpr::isExplicit function. +/// FIXME: Remove when InitListExpr::isExplicit is fixed. +static bool isImplicitILE(const InitListExpr *ILE, const ASTContext &Context) { + // The ILE is implicit if either: + // - The left brace loc of the ILE matches the start of first init expression + // (for non designated decls) + // - The right brace loc of the ILE matches the end of first init expression + // (for designated decls) + // The first init expression should be taken from the syntactic form, but + // since the ILE could be implicit, there might not be a syntactic form. + // For that reason we have to check against all init expressions. + for (const Expr *Init : ILE->inits()) { + if (ILE->getLBraceLoc() == Init->getBeginLoc() || + ILE->getRBraceLoc() == Init->getEndLoc()) + return true; + } + return false; +} + +/// Finds the semantic form of the first explicit ancestor of the given +/// initializer list including itself. +static const InitListExpr *getExplicitILE(const InitListExpr *ILE, + ASTContext &Context) { + if (!isImplicitILE(ILE, Context)) + return ILE; + const InitListExpr *TopLevelILE = ILE; + DynTypedNodeList Parents = Context.getParents(*TopLevelILE); + while (!Parents.empty() && Parents.begin()->get<InitListExpr>()) { + TopLevelILE = Parents.begin()->get<InitListExpr>(); + Parents = Context.getParents(*TopLevelILE); + if (!isImplicitILE(TopLevelILE, Context)) + break; + } + if (!TopLevelILE->isSemanticForm()) { + return TopLevelILE->getSemanticForm(); + } + return TopLevelILE; +} + +static void reportError(const Twine &Message, SourceLocation Loc, + const SourceManager &SM) { + if (Loc.isValid()) { + llvm::errs() << SM.getFilename(Loc) << ":" << SM.getPresumedLineNumber(Loc) + << ":" << SM.getPresumedColumnNumber(Loc) << ": "; + } + llvm::errs() << Message; +} + /// Reorders initializers in the brace initialization of an aggregate. /// /// At the moment partial initialization is not supported. /// \returns true on success static bool reorderFieldsInInitListExpr( - const InitListExpr *InitListEx, ArrayRef<unsigned> NewFieldsOrder, - const ASTContext &Context, + const InitListExpr *InitListEx, const ReorderedStruct &RS, + ASTContext &Context, std::map<std::string, tooling::Replacements> &Replacements) { assert(InitListEx && "Init list expression is null"); - // We care only about InitListExprs which originate from source code. - // Implicit InitListExprs are created by the semantic analyzer. - if (!InitListEx->isExplicit()) + // Only process semantic forms of initializer lists. + if (!InitListEx->isSemanticForm()) { return true; - // The method InitListExpr::getSyntacticForm may return nullptr indicating - // that the current initializer list also serves as its syntactic form. - if (const auto *SyntacticForm = InitListEx->getSyntacticForm()) - InitListEx = SyntacticForm; + } + // If there are no initializers we do not need to change anything. if (!InitListEx->getNumInits()) return true; - if (InitListEx->getNumInits() != NewFieldsOrder.size()) { - llvm::errs() << "Currently only full initialization is supported\n"; - return false; + + // We care only about InitListExprs which originate from source code. + // Implicit InitListExprs are created by the semantic analyzer. + // We find the first parent InitListExpr that exists in source code and + // process it. This is necessary because of designated initializer lists and + // possible omitted braces. + InitListEx = getExplicitILE(InitListEx, Context); + + // Find if there are any designated initializations or implicit values. If all + // initializers are present and none have designators then just reorder them + // normally. Otherwise, designators are added to all initializers and they are + // sorted in the new order. + bool HasImplicitInit = false; + bool HasDesignatedInit = false; + // The method InitListExpr::getSyntacticForm may return nullptr indicating + // that the current initializer list also serves as its syntactic form. + const InitListExpr *SyntacticInitListEx = InitListEx; + if (const InitListExpr *SynILE = InitListEx->getSyntacticForm()) { + // Do not rewrite zero initializers. This check is only valid for syntactic + // forms. + if (SynILE->isIdiomaticZeroInitializer(Context.getLangOpts())) + return true; + + HasImplicitInit = InitListEx->getNumInits() != SynILE->getNumInits(); + HasDesignatedInit = llvm::any_of(SynILE->inits(), [](const Expr *Init) { + return isa<DesignatedInitExpr>(Init); + }); + + SyntacticInitListEx = SynILE; + } else { + // If there is no syntactic form, there can be no designators. Instead, + // there might be implicit values. + HasImplicitInit = + (RS.NewFieldsOrder.size() != InitListEx->getNumInits()) || + llvm::any_of(InitListEx->inits(), [&Context](const Expr *Init) { + return isa<ImplicitValueInitExpr>(Init) || + (isa<InitListExpr>(Init) && + isImplicitILE(dyn_cast<InitListExpr>(Init), Context)); + }); + } + + if (HasImplicitInit || HasDesignatedInit) { + // Designators are only supported from C++20. + if (!HasDesignatedInit && Context.getLangOpts().CPlusPlus && + !Context.getLangOpts().CPlusPlus20) { + reportError( + "Only full initialization without implicit values is supported\n", + InitListEx->getBeginLoc(), Context.getSourceManager()); + return false; + } + + // Handle case when some fields are designated. Some fields can be + // missing. Insert any missing designators and reorder the expressions + // according to the new order. + std::optional<Designators> CurrentDesignators; + // Remember each initializer expression along with its designators. They are + // sorted later to determine the correct order. + std::vector<std::pair<Designators, const Expr *>> Rewrites; + for (const Expr *Init : SyntacticInitListEx->inits()) { + if (const auto *DIE = dyn_cast_or_null<DesignatedInitExpr>(Init)) { + CurrentDesignators.emplace(DIE, SyntacticInitListEx, &Context); + if (!CurrentDesignators->isValid()) { + reportError("Unsupported initializer list\n", DIE->getBeginLoc(), + Context.getSourceManager()); + return false; + } + + // Use the child of the DesignatedInitExpr. This way designators are + // always replaced. + Rewrites.emplace_back(*CurrentDesignators, DIE->getInit()); + } else { + // If designators are not initialized then initialize to the first + // field, otherwise move the next field. + if (!CurrentDesignators) { + CurrentDesignators.emplace(Init, SyntacticInitListEx, &Context); + if (!CurrentDesignators->isValid()) { + reportError("Unsupported initializer list\n", + InitListEx->getBeginLoc(), Context.getSourceManager()); + return false; + } + } else if (!CurrentDesignators->advanceToNextField(Init)) { + reportError("Unsupported initializer list\n", + InitListEx->getBeginLoc(), Context.getSourceManager()); + return false; + } + + // Do not rewrite implicit values. They just had to be processed to + // find the correct designator. + if (!isa<ImplicitValueInitExpr>(Init)) + Rewrites.emplace_back(*CurrentDesignators, Init); + } + } + + // Sort the designators according to the new order. + llvm::stable_sort(Rewrites, [&RS](const auto &Lhs, const auto &Rhs) { + return RS(Lhs.first, Rhs.first); + }); + + for (unsigned i = 0, e = Rewrites.size(); i < e; ++i) { + addReplacement(SyntacticInitListEx->getInit(i)->getSourceRange(), + Rewrites[i].second->getSourceRange(), + Rewrites[i].first.toString(), Context, Replacements); + } + } else { + // Handle excess initializers by leaving them unchanged. + assert(SyntacticInitListEx->getNumInits() >= InitListEx->getNumInits()); + + // All field initializers are present and none have designators. They can be + // reordered normally. + for (unsigned i = 0, e = RS.NewFieldsOrder.size(); i < e; ++i) { + if (i != RS.NewFieldsOrder[i]) + addReplacement(SyntacticInitListEx->getInit(i)->getSourceRange(), + SyntacticInitListEx->getInit(RS.NewFieldsOrder[i]) + ->getSourceRange(), + Context, Replacements); + } } - for (unsigned i = 0, e = InitListEx->getNumInits(); i < e; ++i) - if (i != NewFieldsOrder[i]) - addReplacement(InitListEx->getInit(i)->getSourceRange(), - InitListEx->getInit(NewFieldsOrder[i])->getSourceRange(), - Context, Replacements); return true; } @@ -432,7 +664,9 @@ class ReorderingConsumer : public ASTConsumer { getNewFieldsOrder(RD, DesiredFieldsOrder); if (NewFieldsOrder.empty()) return; - if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements)) + ReorderedStruct RS{RD, NewFieldsOrder}; + + if (!reorderFieldsInDefinition(RS, Context, Replacements)) return; // CXXRD will be nullptr if C code (not C++) is being processed. @@ -440,24 +674,25 @@ class ReorderingConsumer : public ASTConsumer { if (CXXRD) for (const auto *C : CXXRD->ctors()) if (const auto *D = dyn_cast<CXXConstructorDecl>(C->getDefinition())) - reorderFieldsInConstructor(cast<const CXXConstructorDecl>(D), - NewFieldsOrder, Context, Replacements); + reorderFieldsInConstructor(cast<const CXXConstructorDecl>(D), RS, + Context, Replacements); // We only need to reorder init list expressions for // plain C structs or C++ aggregate types. // For other types the order of constructor parameters is used, // which we don't change at the moment. // Now (v0) partial initialization is not supported. - if (!CXXRD || CXXRD->isAggregate()) + if (!CXXRD || CXXRD->isAggregate()) { for (auto Result : match(initListExpr(hasType(equalsNode(RD))).bind("initListExpr"), Context)) if (!reorderFieldsInInitListExpr( - Result.getNodeAs<InitListExpr>("initListExpr"), NewFieldsOrder, - Context, Replacements)) { + Result.getNodeAs<InitListExpr>("initListExpr"), RS, Context, + Replacements)) { Replacements.clear(); return; } + } } }; } // end anonymous namespace diff --git a/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.c b/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.c new file mode 100644 index 0000000000000..7b531d9230c79 --- /dev/null +++ b/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.c @@ -0,0 +1,12 @@ +// RUN: clang-reorder-fields -record-name Foo -fields-order z,y,x %s -- | FileCheck %s + +struct Foo { + int x; // CHECK: {{^ int z;}} + int y; // CHECK-NEXT: {{^ int y;}} + int z; // CHECK-NEXT: {{^ int x;}} +}; + +int main() { + struct Foo foo = { 0, 1 }; // CHECK: {{^ struct Foo foo = { .y = 1, .x = 0 };}} + return 0; +} diff --git a/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.cpp b/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.cpp index 9d09c8187751c..cda5f56e3ce8d 100644 --- a/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.cpp +++ b/clang-tools-extra/test/clang-reorder-fields/AggregatePartialInitialization.cpp @@ -1,4 +1,5 @@ -// RUN: clang-reorder-fields -record-name Foo -fields-order z,y,x %s -- | FileCheck %s +// RUN: clang-reorder-fields --extra-arg="-std=c++17" -record-name Foo -fields-order z,y,x %s -- 2>&1 | FileCheck --check-prefix=CHECK-MESSAGES %s +// RUN: clang-reorder-fields --extra-arg="-std=c++17" -record-name Foo -fields-order z,y,x %s -- | FileCheck %s // The order of fields should not change. class Foo { @@ -9,6 +10,7 @@ class Foo { }; int main() { + // CHECK-MESSAGES: :[[@LINE+1]]:13: Only full initialization without implicit values is supported Foo foo = { 0, 1 }; // CHECK: {{^ Foo foo = { 0, 1 };}} return 0; } diff --git a/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c b/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c new file mode 100644 index 0000000000000..c05c296d1b47b --- /dev/null +++ b/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c @@ -0,0 +1,31 @@ +// RUN: clang-reorder-fields -record-name Foo -fields-order z,w,y,x %s -- | FileCheck %s + +struct Foo { + const int* x; // CHECK: {{^ double z;}} + int y; // CHECK-NEXT: {{^ int w;}} + double z; // CHECK-NEXT: {{^ int y;}} + int w; // CHECK-NEXT: {{^ const int\* x}} +}; + +struct Bar { + char a; + struct Foo b; + char c; +}; + +int main() { + const int x = 13; + struct Foo foo1 = { .x=&x, .y=0, .z=1.29, .w=17 }; // CHECK: {{^ struct Foo foo1 = { .z = 1.29, .w = 17, .y = 0, .x = &x };}} + struct Foo foo2 = { .x=&x, 0, 1.29, 17 }; // CHECK: {{^ struct Foo foo2 = { .z = 1.29, .w = 17, .y = 0, .x = &x };}} + struct Foo foo3 = { .y=0, .z=1.29, 17, .x=&x }; // CHECK: {{^ struct Foo foo3 = { .z = 1.29, .w = 17, .y = 0, .x = &x };}} + struct Foo foo4 = { .y=0, .z=1.29, 17 }; // CHECK: {{^ struct Foo foo4 = { .z = 1.29, .w = 17, .y = 0 };}} + + struct Foo foos1[1] = { [0] = {.x=&x, 0, 1.29, 17} }; // CHECK: {{^ struct Foo foos1\[1] = { \[0] = {.z = 1.29, .w = 17, .y = 0, .x = &x} };}} + struct Foo foos2[1] = { [0].x=&x, [0].y=0, [0].z=1.29, [0].w=17 }; // CHECK: {{^ struct Foo foos2\[1] = { \[0].z = 1.29, \[0].w = 17, \[0].y = 0, \[0].x = &x };}} + struct Foo foos3[1] = { &x, 0, 1.29, 17 }; // CHECK: {{^ struct Foo foos3\[1] = { \[0].z = 1.29, \[0].w = 17, \[0].y = 0, \[0].x = &x };}} + struct Foo foos4[2] = { &x, 0, 1.29, 17, &x, 0, 1.29, 17 }; // CHECK: {{^ struct Foo foos4\[2] = { \[0].z = 1.29, \[0].w = 17, \[0].y = 0, \[0].x = &x, \[1].z = 1.29, \[1].w = 17, \[1].y = 0, \[1].x = &x };}} + + struct Bar bar1 = { .a='a', &x, 0, 1.29, 17, 'c' }; // CHECK: {{^ struct Bar bar1 = { .a = 'a', .b.z = 1.29, .b.w = 17, .b.y = 0, .b.x = &x, .c = 'c' };}} + + return 0; +} diff --git a/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.cpp b/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.cpp new file mode 100644 index 0000000000000..15dac775b6115 --- /dev/null +++ b/clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.cpp @@ -0,0 +1,24 @@ +// RUN: clang-reorder-fields --extra-arg="-std=c++20" -record-name Bar -fields-order c,a,b %s -- | FileCheck %s + +class Foo { +public: + const int* x; + int y; +}; + +class Bar { +public: + char a; // CHECK: {{^ int c;}} + Foo b; // CHECK-NEXT: {{^ char a;}} + int c; // CHECK-NEXT: {{^ Foo b;}} +}; + +int main() { + const int x = 13; + Bar bar1 = { 'a', { &x, 0 }, 123 }; // CHECK: {{^ Bar bar1 = { 123, 'a', { &x, 0 } };}} + Bar bar2 = { .a = 'a', { &x, 0 }, 123 }; // CHECK: {{^ Bar bar2 = { .c = 123, .a = 'a', .b = { &x, 0 } };}} + Bar bar3 = { 'a', .b { &x, 0 }, 123 }; // CHECK: {{^ Bar bar3 = { .c = 123, .a = 'a', .b = { &x, 0 } };}} + Bar bar4 = { .c = 123, .b { &x, 0 }, .a = 'a' }; // CHECK: {{^ Bar bar4 = { .c = 123, .a = 'a', .b = { &x, 0 } };}} + + return 0; +} diff --git a/clang-tools-extra/test/clang-reorder-fields/IdiomaticZeroInitializer.c b/clang-tools-extra/test/clang-reorder-fields/IdiomaticZeroInitializer.c new file mode 100644 index 0000000000000..59c12e81afc0a --- /dev/null +++ b/clang-tools-extra/test/clang-reorder-fields/IdiomaticZeroInitializer.c @@ -0,0 +1,14 @@ +// RUN: clang-reorder-fields -record-name Foo -fields-order y,x %s -- | FileCheck %s + +struct Foo { + int x; // CHECK: {{^ int y;}} + int y; // CHECK-NEXT: {{^ int x;}} +}; + +int main() { + // The idiomatic zero initializer should remain the same. + struct Foo foo0 = { 0 }; // CHECK: {{^ struct Foo foo0 = { 0 };}} + struct Foo foo1 = { 1 }; // CHECK: {{^ struct Foo foo1 = { .x = 1 };}} + + return 0; +} diff --git a/clang-tools-extra/test/clang-reorder-fields/InitializerListExcessElements.c b/clang-tools-extra/test/clang-reorder-fields/InitializerListExcessElements.c new file mode 100644 index 0000000000000..b626e84fea4a3 --- /dev/null +++ b/clang-tools-extra/test/clang-reorder-fields/InitializerListExcessElements.c @@ -0,0 +1,15 @@ +// RUN: clang-reorder-fields -record-name Foo -fields-order y,x %s -- 2>&1 | FileCheck --check-prefix=CHECK-MESSAGES %s +// RUN: clang-reorder-fields -record-name Foo -fields-order y,x %s -- | FileCheck %s + +// The order of fields should not change. +struct Foo { + int x; // CHECK: {{^ int x;}} + int y; // CHECK-NEXT: {{^ int y;}} +}; + +int main() { + // CHECK-MESSAGES: :[[@LINE+1]]:20: Unsupported initializer list + struct Foo foo = { .y=9, 123, .x=1 }; // CHECK: {{^ struct Foo foo = { .y=9, 123, .x=1 };}} + + return 0; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
