Diagnose more cases of initializing distinct members of an anonymous union

member.  Use a better diagnostic for this case.  Also fix a bug with nested
anonymous structs/unions for -Wreorder;  this last was PR6575.

llvm-svn: 100923

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2461,10 +2461,11 @@ def err_overload_incorrect_fntype : Error<
 def err_only_constructors_take_base_inits : Error<
   "only constructors take base initializers">;
 
-def error_multiple_mem_initialization : Error <
-  "multiple initializations given for non-static member '%0'">;
-
-def error_multiple_base_initialization : Error <
+def err_multiple_mem_initialization : Error <
+  "multiple initializations given for non-static member %0">;
+def err_multiple_mem_union_initialization : Error <
+  "initializing multiple members of anonymous union">;
+def err_multiple_base_initialization : Error <
   "multiple initializations given for base %0">;
 
 def err_mem_init_not_member_or_class : Error<

clang/lib/Sema/SemaDeclCXX.cpp

@@ -1660,11 +1660,20 @@ static void *GetKeyForMember(ASTContext &Context,
   if (MemberMaybeAnon && Field->isAnonymousStructOrUnion())
     Field = Member->getAnonUnionMember();
   
-  // If the field is a member of an anonymous union, we use record decl of the
-  // union as the key.
+  // If the field is a member of an anonymous struct or union, our key
+  // is the anonymous record decl that's a direct child of the class.
   RecordDecl *RD = Field->getParent();
-  if (RD->isAnonymousStructOrUnion() && RD->isUnion())
+  if (RD->isAnonymousStructOrUnion()) {
+    while (true) {
+      RecordDecl *Parent = cast<RecordDecl>(RD->getDeclContext());
+      if (Parent->isAnonymousStructOrUnion())
+        RD = Parent;
+      else
+        break;
+    }
+      
     return static_cast<void *>(RD);
+  }
 
   return static_cast<void *>(Field);
 }
@@ -1754,6 +1763,71 @@ DiagnoseBaseOrMemInitializerOrder(Sema &SemaRef,
   }
 }
 
+namespace {
+bool CheckRedundantInit(Sema &S,
+                        CXXBaseOrMemberInitializer *Init,
+                        CXXBaseOrMemberInitializer *&PrevInit) {
+  if (!PrevInit) {
+    PrevInit = Init;
+    return false;
+  }
+
+  if (FieldDecl *Field = Init->getMember())
+    S.Diag(Init->getSourceLocation(),
+           diag::err_multiple_mem_initialization)
+      << Field->getDeclName()
+      << Init->getSourceRange();
+  else {
+    Type *BaseClass = Init->getBaseClass();
+    assert(BaseClass && "neither field nor base");
+    S.Diag(Init->getSourceLocation(),
+           diag::err_multiple_base_initialization)
+      << QualType(BaseClass, 0)
+      << Init->getSourceRange();
+  }
+  S.Diag(PrevInit->getSourceLocation(), diag::note_previous_initializer)
+    << 0 << PrevInit->getSourceRange();
+
+  return true;
+}
+
+typedef std::pair<NamedDecl *, CXXBaseOrMemberInitializer *> UnionEntry;
+typedef llvm::DenseMap<RecordDecl*, UnionEntry> RedundantUnionMap;
+
+bool CheckRedundantUnionInit(Sema &S,
+                             CXXBaseOrMemberInitializer *Init,
+                             RedundantUnionMap &Unions) {
+  FieldDecl *Field = Init->getMember();
+  RecordDecl *Parent = Field->getParent();
+  if (!Parent->isAnonymousStructOrUnion())
+    return false;
+
+  NamedDecl *Child = Field;
+  do {
+    if (Parent->isUnion()) {
+      UnionEntry &En = Unions[Parent];
+      if (En.first && En.first != Child) {
+        S.Diag(Init->getSourceLocation(),
+               diag::err_multiple_mem_union_initialization)
+          << Field->getDeclName()
+          << Init->getSourceRange();
+        S.Diag(En.second->getSourceLocation(), diag::note_previous_initializer)
+          << 0 << En.second->getSourceRange();
+        return true;
+      } else if (!En.first) {
+        En.first = Child;
+        En.second = Init;
+      }
+    }
+
+    Child = Parent;
+    Parent = cast<RecordDecl>(Parent->getDeclContext());
+  } while (Parent->isAnonymousStructOrUnion());
+
+  return false;
+}
+}
+
 /// ActOnMemInitializers - Handle the member initializers for a constructor.
 void Sema::ActOnMemInitializers(DeclPtrTy ConstructorDecl,
                                 SourceLocation ColonLoc,
@@ -1774,34 +1848,29 @@ void Sema::ActOnMemInitializers(DeclPtrTy ConstructorDecl,
   
   CXXBaseOrMemberInitializer **MemInits =
     reinterpret_cast<CXXBaseOrMemberInitializer **>(meminits);
-  
+
+  // Mapping for the duplicate initializers check.
+  // For member initializers, this is keyed with a FieldDecl*.
+  // For base initializers, this is keyed with a Type*.
   llvm::DenseMap<void*, CXXBaseOrMemberInitializer *> Members;
+
+  // Mapping for the inconsistent anonymous-union initializers check.
+  RedundantUnionMap MemberUnions;
+
   bool HadError = false;
   for (unsigned i = 0; i < NumMemInits; i++) {
-    CXXBaseOrMemberInitializer *Member = MemInits[i];
+    CXXBaseOrMemberInitializer *Init = MemInits[i];
 
-    void *KeyToMember = GetKeyForMember(Context, Member);
-    CXXBaseOrMemberInitializer *&PrevMember = Members[KeyToMember];
-    if (!PrevMember) {
-      PrevMember = Member;
-      continue;
+    if (Init->isMemberInitializer()) {
+      FieldDecl *Field = Init->getMember();
+      if (CheckRedundantInit(*this, Init, Members[Field]) ||
+          CheckRedundantUnionInit(*this, Init, MemberUnions))
+        HadError = true;
+    } else {
+      void *Key = GetKeyForBase(Context, QualType(Init->getBaseClass(), 0));
+      if (CheckRedundantInit(*this, Init, Members[Key]))
+        HadError = true;
     }
-    if (FieldDecl *Field = Member->getMember())
-      Diag(Member->getSourceLocation(),
-           diag::error_multiple_mem_initialization)
-        << Field->getNameAsString()
-        << Member->getSourceRange();
-    else {
-      Type *BaseClass = Member->getBaseClass();
-      assert(BaseClass && "ActOnMemInitializers - neither field or base");
-      Diag(Member->getSourceLocation(),
-           diag::error_multiple_base_initialization)
-        << QualType(BaseClass, 0)
-        << Member->getSourceRange();
-    }
-    Diag(PrevMember->getSourceLocation(), diag::note_previous_initializer)
-      << 0;
-    HadError = true;
   }
 
   if (HadError)

clang/test/SemaCXX/class-base-member-init.cpp

@@ -51,3 +51,26 @@ namespace Test3 {
       t(2) { } // expected-error {{multiple initializations given for non-static member 't'}}
   };
 }
+
+namespace test4 {
+  class A {
+    union {
+      struct {
+        int a;
+        int b;
+      };
+
+      int c;
+
+      union {
+        int d;
+        int e;
+      };
+    };
+
+    A(char _) : a(0), b(0) {}
+    A(short _) : a(0), c(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}}
+    A(int _) : d(0), e(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}}
+    A(long _) : a(0), d(0) {} // expected-error {{initializing multiple members of anonymous union}} expected-note {{previous initialization is here}}
+  };
+}

clang/test/SemaCXX/constructor-initializer.cpp

@@ -73,8 +73,9 @@ class U {
   union { int a; char* p; };
   union { int b; double d; };
 
-  U() :  a(1), p(0), d(1.0)  {} // expected-error {{multiple initializations given for non-static member 'p'}} \
-                        // expected-note {{previous initialization is here}}
+  U() :  a(1), // expected-note {{previous initialization is here}}
+         p(0), // expected-error {{initializing multiple members of anonymous union}}
+         d(1.0)  {}
 };
 
 struct V {};

clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp

@@ -108,3 +108,15 @@ namespace test2 {
     Foo y;
   };
 }
+
+// PR6575: this should not crash
+namespace test3 {
+  struct MyClass {
+    MyClass() : m_int(0) {}
+    union {
+      struct {
+        int m_int;
+      };
+    };
+  };
+}