ayzhao created this revision. ayzhao added a reviewer: shafik. Herald added a project: All. ayzhao requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Additional fixes for review comments made in https://reviews.llvm.org/D129531 after the patch was submitted. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D140159 Files: clang/include/clang/AST/ExprCXX.h clang/lib/AST/ExprConstant.cpp clang/lib/CodeGen/CGExprAgg.cpp clang/lib/Sema/SemaInit.cpp clang/lib/Serialization/ASTReaderStmt.cpp clang/lib/Serialization/ASTWriterStmt.cpp clang/test/PCH/cxx_paren_init.cpp clang/test/PCH/cxx_paren_init.h
Index: clang/test/PCH/cxx_paren_init.h =================================================================== --- clang/test/PCH/cxx_paren_init.h +++ clang/test/PCH/cxx_paren_init.h @@ -1,5 +1,9 @@ struct S { int i, j; }; -constexpr S foo(int i, int j) { return S(i, j); }; +union U { unsigned : 8; int i; char j; }; -void bar(int i, int j) { int arr[4](i, j); }; +constexpr S foo(int i, int j) { return S(i, j); } + +void bar(int i, int j) { int arr[4](i, j); } + +constexpr U baz(int i) { return U(i); } Index: clang/test/PCH/cxx_paren_init.cpp =================================================================== --- clang/test/PCH/cxx_paren_init.cpp +++ clang/test/PCH/cxx_paren_init.cpp @@ -5,6 +5,10 @@ // CHECK-DAG: @{{.*s.*}} = {{(dso_local )?}}global [[STRUCT_S]] { i32 1, i32 2 }, align 4 S s = foo(1, 2); +// CHECK-DAG: [[UNION_U:%.*]] = type { i32 } +// CHECK-DAG: @{{.*u.*}} = {{(dso_local )?}}global [[UNION_U]] { i32 3 }, align 4 +U u = baz(3); + // CHECK: define dso_local void @{{.*bar.*}} // CHECK-NEXT: entry: // CHECK-NEXT: [[I_ADDR:%.*]] = alloca i32, align 4 Index: clang/lib/Serialization/ASTWriterStmt.cpp =================================================================== --- clang/lib/Serialization/ASTWriterStmt.cpp +++ clang/lib/Serialization/ASTWriterStmt.cpp @@ -2091,10 +2091,17 @@ Record.AddSourceLocation(E->getEndLoc()); for (Expr *InitExpr : E->getInitExprs()) Record.AddStmt(InitExpr); - bool HasArrayFiller = E->getArrayFiller(); - Record.push_back(HasArrayFiller); - if (HasArrayFiller) - Record.AddStmt(E->getArrayFiller()); + Expr *ArrayFiller = E->getArrayFiller(); + FieldDecl *UnionField = E->getInitializedFieldInUnion(); + bool HasArrayFillerOrUnionDecl = ArrayFiller || UnionField; + Record.push_back(HasArrayFillerOrUnionDecl); + if (HasArrayFillerOrUnionDecl) { + Record.push_back(static_cast<bool>(ArrayFiller)); + if (ArrayFiller) + Record.AddStmt(ArrayFiller); + else + Record.AddDeclRef(UnionField); + } Code = serialization::EXPR_CXX_PAREN_LIST_INIT; } Index: clang/lib/Serialization/ASTReaderStmt.cpp =================================================================== --- clang/lib/Serialization/ASTReaderStmt.cpp +++ clang/lib/Serialization/ASTReaderStmt.cpp @@ -2180,9 +2180,14 @@ for (unsigned I = 0; I < ExpectedNumExprs; I++) E->getTrailingObjects<Expr *>()[I] = Record.readSubExpr(); - bool HasArrayFiller = Record.readBool(); - if (HasArrayFiller) { - E->setArrayFiller(Record.readSubExpr()); + bool HasArrayFillerOrUnionDecl = Record.readBool(); + if (HasArrayFillerOrUnionDecl) { + bool HasArrayFiller = Record.readBool(); + if (HasArrayFiller) { + E->setArrayFiller(Record.readSubExpr()); + } else { + E->setInitilazedFieldInUnion(readDeclAs<FieldDecl>()); + } } E->updateDependence(); } Index: clang/lib/Sema/SemaInit.cpp =================================================================== --- clang/lib/Sema/SemaInit.cpp +++ clang/lib/Sema/SemaInit.cpp @@ -5277,22 +5277,25 @@ SmallVector<Expr *, 4> InitExprs; QualType ResultType; Expr *ArrayFiller = nullptr; + FieldDecl *InitializedFieldInUnion = nullptr; // Process entities (i.e. array members, base classes, or class fields) by // adding an initialization expression to InitExprs for each entity to // initialize. auto ProcessEntities = [&](auto Range) -> bool { + bool IsUnionType = Entity.getType()->isUnionType(); for (InitializedEntity SubEntity : Range) { // Unions should only have one initializer expression. // If there are more initializers than it will be caught when we check // whether Index equals Args.size(). - if (ArgIndexToProcess == 1 && Entity.getType()->isUnionType()) + if (ArgIndexToProcess == 1 && IsUnionType) return true; + bool IsMember = SubEntity.getKind() == InitializedEntity::EK_Member; + // Unnamed bitfields should not be initialized at all, either with an arg // or by default. - if (SubEntity.getKind() == InitializedEntity::EK_Member && - cast<FieldDecl>(SubEntity.getDecl())->isUnnamedBitfield()) + if (IsMember && cast<FieldDecl>(SubEntity.getDecl())->isUnnamedBitfield()) continue; if (ArgIndexToProcess < Args.size()) { @@ -5303,7 +5306,7 @@ // Incomplete array types indicate flexible array members. Do not allow // paren list initializations of structs with these members, as GCC // doesn't either. - if (SubEntity.getKind() == InitializedEntity::EK_Member) { + if (IsMember) { auto *FD = cast<FieldDecl>(SubEntity.getDecl()); if (FD->getType()->isIncompleteArrayType()) { if (!VerifyOnly) { @@ -5333,11 +5336,13 @@ if (!VerifyOnly) { ExprResult ER = SubSeq.Perform(S, SubEntity, SubKind, E); InitExprs.push_back(ER.get()); + if (IsMember && IsUnionType) + InitializedFieldInUnion = cast<FieldDecl>(SubEntity.getDecl()); } } else { // We've processed all of the args, but there are still entities that // have to be initialized. - if (SubEntity.getKind() == InitializedEntity::EK_Member) { + if (IsMember) { // C++ [dcl.init]p17.6.2.2 // The remaining elements are initialized with their default member // initializers, if any @@ -5408,8 +5413,8 @@ return; ResultType = S.Context.getConstantArrayType( - AT->getElementType(), llvm::APInt(32, ArrayLength), nullptr, - ArrayType::Normal, 0); + AT->getElementType(), llvm::APInt(/*numBits=*/32, ArrayLength), + nullptr, ArrayType::Normal, 0); } } else if (auto *RT = Entity.getType()->getAs<RecordType>()) { const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl()); @@ -5451,7 +5456,10 @@ auto *CPLIE = CXXParenListInitExpr::Create( S.getASTContext(), InitExprs, ResultType, Args.size(), Kind.getLocation(), SR.getBegin(), SR.getEnd()); - CPLIE->setArrayFiller(ArrayFiller); + if (ArrayFiller) + CPLIE->setArrayFiller(ArrayFiller); + if (InitializedFieldInUnion) + CPLIE->setInitilazedFieldInUnion(InitializedFieldInUnion); *Result = CPLIE; S.Diag(Kind.getLocation(), diag::warn_cxx17_compat_aggregate_init_paren_list) Index: clang/lib/CodeGen/CGExprAgg.cpp =================================================================== --- clang/lib/CodeGen/CGExprAgg.cpp +++ clang/lib/CodeGen/CGExprAgg.cpp @@ -1601,20 +1601,8 @@ } void AggExprEmitter::VisitCXXParenListInitExpr(CXXParenListInitExpr *E) { - ArrayRef<Expr *> InitExprs = E->getInitExprs(); - FieldDecl *InitializedFieldInUnion = nullptr; - if (E->getType()->isUnionType()) { - auto *RD = - dyn_cast<CXXRecordDecl>(E->getType()->castAs<RecordType>()->getDecl()); - for (FieldDecl *FD : RD->fields()) { - if (FD->isUnnamedBitfield()) - continue; - InitializedFieldInUnion = FD; - break; - } - } - - VisitCXXParenListOrInitListExpr(E, InitExprs, InitializedFieldInUnion, + VisitCXXParenListOrInitListExpr(E, E->getInitExprs(), + E->getInitializedFieldInUnion(), E->getArrayFiller()); } Index: clang/lib/AST/ExprConstant.cpp =================================================================== --- clang/lib/AST/ExprConstant.cpp +++ clang/lib/AST/ExprConstant.cpp @@ -9977,17 +9977,8 @@ const FieldDecl *Field; if (auto *ILE = dyn_cast<InitListExpr>(ExprToVisit)) { Field = ILE->getInitializedFieldInUnion(); - } else if (isa<CXXParenListInitExpr>(ExprToVisit)) { - assert(Args.size() == 1 && - "Unions should have exactly 1 initializer in a C++ paren list"); - - for (const FieldDecl *FD : RD->fields()) { - if (FD->isUnnamedBitfield()) - continue; - - Field = FD; - break; - } + } else if (auto *PLIE = dyn_cast<CXXParenListInitExpr>(ExprToVisit)) { + Field = PLIE->getInitializedFieldInUnion(); } else { llvm_unreachable( "Expression is neither an init list nor a C++ paren list"); Index: clang/include/clang/AST/ExprCXX.h =================================================================== --- clang/include/clang/AST/ExprCXX.h +++ clang/include/clang/AST/ExprCXX.h @@ -4751,16 +4751,12 @@ unsigned NumExprs; unsigned NumUserSpecifiedExprs; SourceLocation InitLoc, LParenLoc, RParenLoc; - Expr *ArrayFiller = nullptr; + llvm::PointerUnion<Expr *, FieldDecl *> ArrayFillerOrUnionFieldInit; CXXParenListInitExpr(ArrayRef<Expr *> Args, QualType T, unsigned NumUserSpecifiedExprs, SourceLocation InitLoc, SourceLocation LParenLoc, SourceLocation RParenLoc) - : Expr(CXXParenListInitExprClass, T, - T->isLValueReferenceType() ? VK_LValue - : T->isRValueReferenceType() ? VK_XValue - : VK_PRValue, - OK_Ordinary), + : Expr(CXXParenListInitExprClass, T, getValueKindForType(T), OK_Ordinary), NumExprs(Args.size()), NumUserSpecifiedExprs(NumUserSpecifiedExprs), InitLoc(InitLoc), LParenLoc(LParenLoc), RParenLoc(RParenLoc) { std::copy(Args.begin(), Args.end(), getTrailingObjects<Expr *>()); @@ -4815,11 +4811,27 @@ return SourceRange(getBeginLoc(), getEndLoc()); } - void setArrayFiller(Expr *E) { ArrayFiller = E; } + void setArrayFiller(Expr *E) { ArrayFillerOrUnionFieldInit = E; } - Expr *getArrayFiller() { return ArrayFiller; } + Expr *getArrayFiller() { + return ArrayFillerOrUnionFieldInit.dyn_cast<Expr *>(); + } - const Expr *getArrayFiller() const { return ArrayFiller; } + const Expr *getArrayFiller() const { + return ArrayFillerOrUnionFieldInit.dyn_cast<Expr *>(); + } + + void setInitilazedFieldInUnion(FieldDecl *FD) { + ArrayFillerOrUnionFieldInit = FD; + } + + FieldDecl *getInitializedFieldInUnion() { + return ArrayFillerOrUnionFieldInit.dyn_cast<FieldDecl *>(); + } + + const FieldDecl *getInitializedFieldInUnion() const { + return ArrayFillerOrUnionFieldInit.dyn_cast<FieldDecl *>(); + } child_range children() { Stmt **Begin = reinterpret_cast<Stmt **>(getTrailingObjects<Expr *>());
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits