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
  • [PATCH] D140159: [clang][nfc] Fi... Alan Zhao via Phabricator via cfe-commits

Reply via email to