[clang][nfc] Fixes for D129531

[ayzhao]

Summary

Additional fixes for review comments made in
https://reviews.llvm.org/D129531 after the patch was submitted.

[ayzhao]

Abandoning as the underlying commit was reverted.

clang/include/clang/AST/ExprCXX.h

: public Expr,
       private llvm::TrailingObjects<CXXParenListInitExpr, Expr *> {
   friend class TrailingObjects;
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
   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 *>());
     assert(NumExprs >= NumUserSpecifiedExprs &&
            "number of user specified inits is greater than the number of "
            "passed inits");
     setDependence(computeDependence(this));
   }
public:
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
 
   SourceLocation getInitLoc() const LLVM_READONLY { return InitLoc; }
 
   SourceRange getSourceRange() const LLVM_READONLY {
     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 *>());
     return child_range(Begin, Begin + NumExprs);
   }
 
   const_child_range children() const {
     Stmt *const *Begin =

clang/lib/AST/ExprConstant.cpp

EvalInfo::EvaluatingConstructorRAII EvalObj(
       Info,
       ObjectUnderConstruction{This.getLValueBase(), This.Designator.Entries},
       CXXRD && CXXRD->getNumBases());
 
   if (RD->isUnion()) {
     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");
     }
 
     Result = APValue(Field);
     if (!Field)
       return true;

clang/lib/CodeGen/CGExprAgg.cpp

if (CGF.hasScalarEvaluationKind(type)) {
     // There's a potential optimization opportunity in combining
     // memsets; that would be easy for arrays, but relatively
     // difficult for structures with the current code.
     CGF.EmitNullInitialization(lv.getAddress(CGF), lv.getType());
   }
 }
 
 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());
 }
 
 void AggExprEmitter::VisitInitListExpr(InitListExpr *E) {
   if (E->hadArrayRangeDesignator())
     CGF.ErrorUnsupported(E, "GNU array range designator extension");
 
   if (E->isTransparent())

clang/lib/Sema/SemaInit.cpp

 static void TryOrBuildParenListInitialization(
     Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
     ArrayRef<Expr *> Args, InitializationSequence &Sequence, bool VerifyOnly,
     ExprResult *Result = nullptr) {
   unsigned ArgIndexToProcess = 0;
   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()) {
         // There are still expressions in Args that haven't been processed.
         // Let's match them to the current entity to initialize.
         Expr *E = Args[ArgIndexToProcess++];
 
         // 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) {
               S.Diag(E->getBeginLoc(), diag::err_flexible_array_init)
                   << SourceRange(E->getBeginLoc(), E->getEndLoc());
               S.Diag(FD->getLocation(), diag::note_flexible_array_member) << FD;
             }
             Sequence.SetFailed(
for (InitializedEntity SubEntity : Range) {
             Sequence.SetFailed(
                 InitializationSequence::FK_ParenthesizedListInitFailed);
 
           return false;
         }
         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
           auto *FD = cast<FieldDecl>(SubEntity.getDecl());
           if (Expr *ICE = FD->getInClassInitializer(); ICE && !VerifyOnly) {
             ExprResult DIE = S.BuildCXXDefaultInitExpr(FD->getLocation(), FD);
             if (DIE.isInvalid())
               return false;
if (ArrayLength >= Args.size()) {
       for (uint64_t I = 0; I < ArrayLength; ++I)
         ElementEntities.push_back(
             InitializedEntity::InitializeElement(S.getASTContext(), I, Entity));
 
       if (!ProcessEntities(ElementEntities))
         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());
 
     auto BaseRange = map_range(RD->bases(), [&S](auto &base) {
       return InitializedEntity::InitializeBase(S.getASTContext(), &base, false);
     });
     auto FieldRange = map_range(RD->fields(), [](auto *field) {
static void TryOrBuildParenListInitialization(
   if (VerifyOnly) {
     Sequence.setSequenceKind(InitializationSequence::NormalSequence);
     Sequence.AddParenthesizedListInitStep(Entity.getType());
   } else if (Result) {
     SourceRange SR = Kind.getParenOrBraceRange();
     auto *CPLIE = CXXParenListInitExpr::Create(
         S.getASTContext(), InitExprs, ResultType, Args.size(),
         Kind.getLocation(), SR.getBegin(), SR.getEnd());
+    if (ArrayFiller)
       CPLIE->setArrayFiller(ArrayFiller);
+    if (InitializedFieldInUnion)
+      CPLIE->setInitilazedFieldInUnion(InitializedFieldInUnion);
     *Result = CPLIE;
     S.Diag(Kind.getLocation(),
            diag::warn_cxx17_compat_aggregate_init_paren_list)
         << Kind.getLocation() << SR << ResultType;
   }
 
   return;
 }

clang/lib/Serialization/ASTReaderStmt.cpp

assert(E->NumExprs == ExpectedNumExprs &&
          "serialized expressions.");
   E->NumUserSpecifiedExprs = Record.readInt();
   E->InitLoc = readSourceLocation();
   E->LParenLoc = readSourceLocation();
   E->RParenLoc = readSourceLocation();
   for (unsigned I = 0; I < ExpectedNumExprs; I++)
     E->getTrailingObjects<Expr *>()[I] = 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();
 }
 
 void ASTStmtReader::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   VisitExpr(E);
   E->SourceExpr = Record.readSubExpr();
   E->OpaqueValueExprBits.Loc = readSourceLocation();

clang/lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitCXXParenListInitExpr(CXXParenListInitExpr *E) {
   ArrayRef<Expr *> InitExprs = E->getInitExprs();
   Record.push_back(InitExprs.size());
   Record.push_back(E->getUserSpecifiedInitExprs().size());
   Record.AddSourceLocation(E->getInitLoc());
   Record.AddSourceLocation(E->getBeginLoc());
   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;
 }
 
 void ASTStmtWriter::VisitOpaqueValueExpr(OpaqueValueExpr *E) {
   VisitExpr(E);
   Record.AddStmt(E->getSourceExpr());
   Record.AddSourceLocation(E->getLocation());
   Record.push_back(E->isUnique());

clang/test/PCH/cxx_paren_init.h

 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); }

clang/test/PCH/cxx_paren_init.cpp

 // RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-linux-gnu -emit-pch -o %t %S/cxx_paren_init.h
 // RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-linux-gnu -include-pch %t %s -S -emit-llvm -o - | FileCheck %s
 
 // CHECK-DAG: [[STRUCT_S:%.*]] = type { i32, i32 }
 // 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
 // CHECK-NEXT: [[J_ADDR:%.*]] = alloca i32, align 4
 // CHECK-NEXT: [[ARR:%.*]] = alloca [4 x i32], align 16
 // CHECK-NEXT: store i32 [[A:%.*]], ptr [[I_ADDR]], align 4
 // CHECK-NEXT: store i32 [[B:%.*]], ptr [[J_ADDR]], align 4
 // CHECK-NEXT: [[ARRINIT_BEGIN:%.*]] = getelementptr inbounds [4 x i32], ptr [[ARR]], i64 0, i64 0