[cfe-commits] r107242 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/constructor-initializer.cpp

[Chandler Carruth]

Author: chandlerc
Date: Tue Jun 29 21:59:29 2010
New Revision: 107242

URL: http://llvm.org/viewvc/llvm-project?rev=107242&view=rev
Log:
Reapply r107235, this time with both my typo fixed, and a logical bug fixed.
Previously we relied on the presence of a member which needs no initialization
to prevent us from creating an additional initialization of the outer anonymous
union field. We have already correctly marked that field as initialized by the
member of the union (repeatedly due to the original bug this patch fixes) so we
simply need to bail out.

Modified:
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/SemaCXX/constructor-initializer.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=107242&r1=107241&r2=107242&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Jun 29 21:59:29 2010
@@ -1753,38 +1753,67 @@
 };
 }
 
+static void RecordFieldInitializer(BaseAndFieldInfo &Info,
+                                   FieldDecl *Top, FieldDecl *Field,
+                                   CXXBaseOrMemberInitializer *Init) {
+  // If the member doesn't need to be initialized, Init will still be null.
+  if (!Init)
+    return;
+
+  Info.AllToInit.push_back(Init);
+  if (Field != Top) {
+    Init->setMember(Top);
+    Init->setAnonUnionMember(Field);
+  }
+}
+
 static bool CollectFieldInitializer(BaseAndFieldInfo &Info,
                                     FieldDecl *Top, FieldDecl *Field) {
 
-  // Overwhelmingly common case:  we have a direct initializer for this field.
+  // Overwhelmingly common case: we have a direct initializer for this field.
   if (CXXBaseOrMemberInitializer *Init = Info.AllBaseFields.lookup(Field)) {
-    Info.AllToInit.push_back(Init);
-
-    if (Field != Top) {
-      Init->setMember(Top);
-      Init->setAnonUnionMember(Field);
-    }
+    RecordFieldInitializer(Info, Top, Field, Init);
     return false;
   }
 
   if (Info.IIK == IIK_Default && Field->isAnonymousStructOrUnion()) {
     const RecordType *FieldClassType = Field->getType()->getAs<RecordType>();
     assert(FieldClassType && "anonymous struct/union without record type");
-
-    // Walk through the members, tying in any initializers for fields
-    // we find.  The earlier semantic checks should prevent redundant
-    // initialization of union members, given the requirement that
-    // union members never have non-trivial default constructors.
-
-    // TODO: in C++0x, it might be legal to have union members with
-    // non-trivial default constructors in unions.  Revise this
-    // implementation then with the appropriate semantics.
     CXXRecordDecl *FieldClassDecl
       = cast<CXXRecordDecl>(FieldClassType->getDecl());
-    for (RecordDecl::field_iterator FA = FieldClassDecl->field_begin(),
-           EA = FieldClassDecl->field_end(); FA != EA; FA++)
-      if (CollectFieldInitializer(Info, Top, *FA))
-        return true;
+
+    // Even though union members never have non-trivial default
+    // constructions in C++03, we still build member initializers for aggregate
+    // record types which can be union members, and C++0x allows non-trivial
+    // default constructors for union members, so we ensure that only one
+    // member is initialized for these.
+    if (FieldClassDecl->isUnion()) {
+      // First check for an explicit initializer for one field.
+      for (RecordDecl::field_iterator FA = FieldClassDecl->field_begin(),
+           EA = FieldClassDecl->field_end(); FA != EA; FA++) {
+        if (CXXBaseOrMemberInitializer *Init = Info.AllBaseFields.lookup(*FA)) {
+          RecordFieldInitializer(Info, Top, *FA, Init);
+
+          // Once we've initialized a field of an anonymous union, the union
+          // field in the class is also initialized, so exit immediately.
+          return false;
+        }
+      }
+
+      // Fallthrough and construct a default initializer for the union as
+      // a whole, which can call its default constructor if such a thing exists
+      // (C++0x perhaps). FIXME: It's not clear that this is the correct
+      // behavior going forward with C++0x, when anonymous unions there are
+      // finalized, we should revisit this.
+    } else {
+      // For structs, we simply descend through to initialize all members where
+      // necessary.
+      for (RecordDecl::field_iterator FA = FieldClassDecl->field_begin(),
+           EA = FieldClassDecl->field_end(); FA != EA; FA++) {
+        if (CollectFieldInitializer(Info, Top, *FA))
+          return true;
+      }
+    }
   }
 
   // Don't try to build an implicit initializer if there were semantic
@@ -1796,15 +1825,8 @@
   CXXBaseOrMemberInitializer *Init = 0;
   if (BuildImplicitMemberInitializer(Info.S, Info.Ctor, Info.IIK, Field, Init))
     return true;
-    
-  // If the member doesn't need to be initialized, Init will still be null.
-  if (!Init) return false;
 
-  Info.AllToInit.push_back(Init);
-  if (Top != Field) {
-    Init->setMember(Top);
-    Init->setAnonUnionMember(Field);
-  }
+  RecordFieldInitializer(Info, Top, Field, Init);
   return false;
 }
                                

Modified: cfe/trunk/test/SemaCXX/constructor-initializer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constructor-initializer.cpp?rev=107242&r1=107241&r2=107242&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constructor-initializer.cpp (original)
+++ cfe/trunk/test/SemaCXX/constructor-initializer.cpp Tue Jun 29 21:59:29 2010
@@ -204,3 +204,20 @@
 }
 
 }
+
+// Don't build implicit initializers for anonymous union fields when we already
+// have an explicit initializer for another field in the union.
+namespace PR7402 {
+  struct S {
+    union {
+      void* ptr_;
+      struct { int i_; };
+    };
+
+    template <typename T> S(T) : ptr_(0) { }
+  };
+
+  void f() {
+    S s(3);
+  }
+}