[cfe-commits] r80934 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/DeclCXX.cpp lib/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp test/SemaCXX/illegal-member-initialization.cpp

Fariborz Jahanian fjahanian at apple.com
Thu Sep 3 12:36:47 PDT 2009


Author: fjahanian
Date: Thu Sep  3 14:36:46 2009
New Revision: 80934

URL: http://llvm.org/viewvc/llvm-project?rev=80934&view=rev
Log:
Issue diagnostics in variety of situations involving
reference/const data members when user has declared
the constructor. This necessitated some non-minor
refactoring.


Added:
    cfe/trunk/test/SemaCXX/illegal-member-initialization.cpp
Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/AST/DeclCXX.cpp
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=80934&r1=80933&r2=80934&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Sep  3 14:36:46 2009
@@ -1039,12 +1039,13 @@
       return NumBaseOrMemberInitializers; 
   }
   
-  void setBaseOrMemberInitializers(ASTContext &C,
-                              CXXBaseOrMemberInitializer **Initializers,
-                              unsigned NumInitializers,
-                              llvm::SmallVectorImpl<CXXBaseSpecifier *>& Bases,
-                              llvm::SmallVectorImpl<FieldDecl *>&Members);
+  void setNumBaseOrMemberInitializers(unsigned numBaseOrMemberInitializers) {
+    NumBaseOrMemberInitializers = numBaseOrMemberInitializers;
+  }
   
+  void setBaseOrMemberInitializers(CXXBaseOrMemberInitializer ** initializers) {
+    BaseOrMemberInitializers = initializers;
+  }
   /// isDefaultConstructor - Whether this constructor is a default
   /// constructor (C++ [class.ctor]p5), which can be used to
   /// default-initialize a class of this type.

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=80934&r1=80933&r2=80934&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep  3 14:36:46 2009
@@ -714,6 +714,12 @@
 def err_unintialized_member : Error<
   "cannot define the implicit default constructor for %0, because "
   "%select{reference|const}1 member %2 cannot be default-initialized">;
+def err_null_intialized_reference_member : Error<
+  "cannot initialize the member to null in default constructor because "
+  "reference member %0 cannot be null-initialized">;
+def err_unintialized_member_in_ctor : Error<
+  "constructor for %0 must explicitly initialize the "
+  "%select{reference|const}1 member %2 ">;
 
 def err_use_of_default_argument_to_function_declared_later : Error<
   "use of default argument to function %0 that is declared later in class %1">;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=80934&r1=80933&r2=80934&view=diff

==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Thu Sep  3 14:36:46 2009
@@ -584,159 +584,6 @@
 }
 
 void
-CXXConstructorDecl::setBaseOrMemberInitializers(
-                                ASTContext &C,
-                                CXXBaseOrMemberInitializer **Initializers,
-                                unsigned NumInitializers,
-                                llvm::SmallVectorImpl<CXXBaseSpecifier *>& Bases,          
-                                llvm::SmallVectorImpl<FieldDecl *>&Fields) {
-  // We need to build the initializer AST according to order of construction
-  // and not what user specified in the Initializers list.
-  CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(getDeclContext());
-  llvm::SmallVector<CXXBaseOrMemberInitializer*, 32> AllToInit;
-  llvm::DenseMap<const void *, CXXBaseOrMemberInitializer*> AllBaseFields;
-  bool HasDependentBaseInit = false;
-  
-  for (unsigned i = 0; i < NumInitializers; i++) {
-    CXXBaseOrMemberInitializer *Member = Initializers[i];
-    if (Member->isBaseInitializer()) {
-      if (Member->getBaseClass()->isDependentType())
-        HasDependentBaseInit = true;
-      AllBaseFields[Member->getBaseClass()->getAs<RecordType>()] = Member;
-    } else {
-      AllBaseFields[Member->getMember()] = Member;
-    }
-  }
-
-  if (HasDependentBaseInit) {
-    // FIXME. This does not preserve the ordering of the initializers.
-    // Try (with -Wreorder)
-    // template<class X> struct A {};
-    // template<class X> struct B : A<X> { 
-    //   B() : x1(10), A<X>() {} 
-    //   int x1;
-    // };
-    // B<int> x;
-    // On seeing one dependent type, we should essentially exit this routine
-    // while preserving user-declared initializer list. When this routine is
-    // called during instantiatiation process, this routine will rebuild the
-    // oderdered initializer list correctly.
-    
-    // If we have a dependent base initialization, we can't determine the
-    // association between initializers and bases; just dump the known
-    // initializers into the list, and don't try to deal with other bases.
-    for (unsigned i = 0; i < NumInitializers; i++) {
-      CXXBaseOrMemberInitializer *Member = Initializers[i];
-      if (Member->isBaseInitializer())
-        AllToInit.push_back(Member);
-    }
-  } else {
-    // Push virtual bases before others.
-    for (CXXRecordDecl::base_class_iterator VBase =
-         ClassDecl->vbases_begin(),
-         E = ClassDecl->vbases_end(); VBase != E; ++VBase) {
-      if (VBase->getType()->isDependentType())
-        continue;
-      if (CXXBaseOrMemberInitializer *Value = 
-          AllBaseFields.lookup(VBase->getType()->getAs<RecordType>()))
-        AllToInit.push_back(Value);
-      else {
-        CXXRecordDecl *VBaseDecl = 
-          cast<CXXRecordDecl>(VBase->getType()->getAs<RecordType>()->getDecl());
-        assert(VBaseDecl && "setBaseOrMemberInitializers - VBaseDecl null");
-        if (!VBaseDecl->getDefaultConstructor(C))
-          Bases.push_back(VBase);
-        CXXBaseOrMemberInitializer *Member = 
-          new (C) CXXBaseOrMemberInitializer(VBase->getType(), 0, 0,
-                                            VBaseDecl->getDefaultConstructor(C),
-                                             SourceLocation(),
-                                             SourceLocation());
-        AllToInit.push_back(Member);
-      }
-    }
-    
-    for (CXXRecordDecl::base_class_iterator Base =
-         ClassDecl->bases_begin(),
-         E = ClassDecl->bases_end(); Base != E; ++Base) {
-      // Virtuals are in the virtual base list and already constructed.
-      if (Base->isVirtual())
-        continue;
-      // Skip dependent types.
-      if (Base->getType()->isDependentType())
-        continue;
-      if (CXXBaseOrMemberInitializer *Value = 
-          AllBaseFields.lookup(Base->getType()->getAs<RecordType>()))
-        AllToInit.push_back(Value);
-      else {
-        CXXRecordDecl *BaseDecl = 
-          cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
-        assert(BaseDecl && "setBaseOrMemberInitializers - BaseDecl null");
-        if (!BaseDecl->getDefaultConstructor(C))
-          Bases.push_back(Base);
-        CXXBaseOrMemberInitializer *Member = 
-        new (C) CXXBaseOrMemberInitializer(Base->getType(), 0, 0,
-                                           BaseDecl->getDefaultConstructor(C),
-                                           SourceLocation(),
-                                           SourceLocation());
-        AllToInit.push_back(Member);
-      }
-    }
-  }
-
-  // non-static data members.
-  for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(),
-       E = ClassDecl->field_end(); Field != E; ++Field) {
-    if ((*Field)->isAnonymousStructOrUnion()) {
-      if (const RecordType *FieldClassType = 
-            Field->getType()->getAs<RecordType>()) {
-        CXXRecordDecl *FieldClassDecl
-          = cast<CXXRecordDecl>(FieldClassType->getDecl());
-        for(RecordDecl::field_iterator FA = FieldClassDecl->field_begin(),
-            EA = FieldClassDecl->field_end(); FA != EA; FA++) {
-          if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(*FA)) {
-            // 'Member' is the anonymous union field and 'AnonUnionMember' is
-            // set to the anonymous union data member used in the initializer
-            // list.
-            Value->setMember(*Field);
-            Value->setAnonUnionMember(*FA);
-            AllToInit.push_back(Value);
-            break;
-          }
-        }
-      }
-      continue;
-    }
-    if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(*Field)) {
-      AllToInit.push_back(Value);
-      continue;
-    }
-
-    QualType FT = C.getBaseElementType((*Field)->getType());
-    if (const RecordType* RT = FT->getAs<RecordType>()) {
-      CXXConstructorDecl *Ctor =
-        cast<CXXRecordDecl>(RT->getDecl())->getDefaultConstructor(C);
-      if (!Ctor && !FT->isDependentType())
-        Fields.push_back(*Field);
-      CXXBaseOrMemberInitializer *Member = 
-        new (C) CXXBaseOrMemberInitializer((*Field), 0, 0,
-                                           Ctor,
-                                           SourceLocation(),
-                                           SourceLocation());
-      AllToInit.push_back(Member);
-    } 
-  }
-
-  NumInitializers = AllToInit.size();
-  if (NumInitializers > 0) {
-    NumBaseOrMemberInitializers = NumInitializers;
-    BaseOrMemberInitializers = 
-      new (C) CXXBaseOrMemberInitializer*[NumInitializers]; 
-    for (unsigned Idx = 0; Idx < NumInitializers; ++Idx)
-      BaseOrMemberInitializers[Idx] = AllToInit[Idx];
-  }
-}
-
-void
 CXXConstructorDecl::Destroy(ASTContext& C) {
   C.Deallocate(BaseOrMemberInitializers);
   CXXMethodDecl::Destroy(C);

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=80934&r1=80933&r2=80934&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Thu Sep  3 14:36:46 2009
@@ -2175,7 +2175,13 @@
                                      unsigned NumArgs, SourceLocation IdLoc,
                                      SourceLocation RParenLoc,
                                      CXXRecordDecl *ClassDecl);
-
+  
+  void setBaseOrMemberInitializers(CXXConstructorDecl *Constructor,
+                              CXXBaseOrMemberInitializer **Initializers,
+                              unsigned NumInitializers,
+                              llvm::SmallVectorImpl<CXXBaseSpecifier *>& Bases,
+                              llvm::SmallVectorImpl<FieldDecl *>&Members);
+  
   void AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl);
 
   virtual void ActOnMemInitializers(DeclPtrTy ConstructorDecl, 

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=80934&r1=80933&r2=80934&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep  3 14:36:46 2009
@@ -792,6 +792,11 @@
   } else if (!HasDependentArg) {
     Expr *NewExp;
     if (NumArgs == 0) {
+      if (FieldType->isReferenceType()) {
+        Diag(IdLoc, diag::err_null_intialized_reference_member)
+              << Member->getDeclName();
+        return Diag(Member->getLocation(), diag::note_declared_at);
+      }
       NewExp = new (Context) CXXZeroInitValueExpr(FieldType, IdLoc, RParenLoc);
       NumArgs = 1;
     }
@@ -890,6 +895,175 @@
 }
 
 void
+Sema::setBaseOrMemberInitializers(CXXConstructorDecl *Constructor,
+                              CXXBaseOrMemberInitializer **Initializers,
+                              unsigned NumInitializers,
+                              llvm::SmallVectorImpl<CXXBaseSpecifier *>& Bases,          
+                              llvm::SmallVectorImpl<FieldDecl *>&Fields) {
+  // We need to build the initializer AST according to order of construction
+  // and not what user specified in the Initializers list.
+  CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(Constructor->getDeclContext());
+  llvm::SmallVector<CXXBaseOrMemberInitializer*, 32> AllToInit;
+  llvm::DenseMap<const void *, CXXBaseOrMemberInitializer*> AllBaseFields;
+  bool HasDependentBaseInit = false;
+  
+  for (unsigned i = 0; i < NumInitializers; i++) {
+    CXXBaseOrMemberInitializer *Member = Initializers[i];
+    if (Member->isBaseInitializer()) {
+      if (Member->getBaseClass()->isDependentType())
+        HasDependentBaseInit = true;
+      AllBaseFields[Member->getBaseClass()->getAs<RecordType>()] = Member;
+    } else {
+      AllBaseFields[Member->getMember()] = Member;
+    }
+  }
+  
+  if (HasDependentBaseInit) {
+    // FIXME. This does not preserve the ordering of the initializers.
+    // Try (with -Wreorder)
+    // template<class X> struct A {};
+    // template<class X> struct B : A<X> { 
+    //   B() : x1(10), A<X>() {} 
+    //   int x1;
+    // };
+    // B<int> x;
+    // On seeing one dependent type, we should essentially exit this routine
+    // while preserving user-declared initializer list. When this routine is
+    // called during instantiatiation process, this routine will rebuild the
+    // oderdered initializer list correctly.
+    
+    // If we have a dependent base initialization, we can't determine the
+    // association between initializers and bases; just dump the known
+    // initializers into the list, and don't try to deal with other bases.
+    for (unsigned i = 0; i < NumInitializers; i++) {
+      CXXBaseOrMemberInitializer *Member = Initializers[i];
+      if (Member->isBaseInitializer())
+        AllToInit.push_back(Member);
+    }
+  } else {
+    // Push virtual bases before others.
+    for (CXXRecordDecl::base_class_iterator VBase =
+         ClassDecl->vbases_begin(),
+         E = ClassDecl->vbases_end(); VBase != E; ++VBase) {
+      if (VBase->getType()->isDependentType())
+        continue;
+      if (CXXBaseOrMemberInitializer *Value = 
+          AllBaseFields.lookup(VBase->getType()->getAs<RecordType>()))
+        AllToInit.push_back(Value);
+      else {
+        CXXRecordDecl *VBaseDecl = 
+        cast<CXXRecordDecl>(VBase->getType()->getAs<RecordType>()->getDecl());
+        assert(VBaseDecl && "setBaseOrMemberInitializers - VBaseDecl null");
+        if (!VBaseDecl->getDefaultConstructor(Context))
+          Bases.push_back(VBase);
+        CXXBaseOrMemberInitializer *Member = 
+        new (Context) CXXBaseOrMemberInitializer(VBase->getType(), 0, 0,
+                                    VBaseDecl->getDefaultConstructor(Context),
+                                    SourceLocation(),
+                                    SourceLocation());
+        AllToInit.push_back(Member);
+      }
+    }
+    
+    for (CXXRecordDecl::base_class_iterator Base =
+         ClassDecl->bases_begin(),
+         E = ClassDecl->bases_end(); Base != E; ++Base) {
+      // Virtuals are in the virtual base list and already constructed.
+      if (Base->isVirtual())
+        continue;
+      // Skip dependent types.
+      if (Base->getType()->isDependentType())
+        continue;
+      if (CXXBaseOrMemberInitializer *Value = 
+          AllBaseFields.lookup(Base->getType()->getAs<RecordType>()))
+        AllToInit.push_back(Value);
+      else {
+        CXXRecordDecl *BaseDecl = 
+        cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
+        assert(BaseDecl && "setBaseOrMemberInitializers - BaseDecl null");
+        if (!BaseDecl->getDefaultConstructor(Context))
+          Bases.push_back(Base);
+        CXXBaseOrMemberInitializer *Member = 
+        new (Context) CXXBaseOrMemberInitializer(Base->getType(), 0, 0,
+                                      BaseDecl->getDefaultConstructor(Context),
+                                      SourceLocation(),
+                                      SourceLocation());
+        AllToInit.push_back(Member);
+      }
+    }
+  }
+  
+  // non-static data members.
+  for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin(),
+       E = ClassDecl->field_end(); Field != E; ++Field) {
+    if ((*Field)->isAnonymousStructOrUnion()) {
+      if (const RecordType *FieldClassType = 
+          Field->getType()->getAs<RecordType>()) {
+        CXXRecordDecl *FieldClassDecl
+        = cast<CXXRecordDecl>(FieldClassType->getDecl());
+        for(RecordDecl::field_iterator FA = FieldClassDecl->field_begin(),
+            EA = FieldClassDecl->field_end(); FA != EA; FA++) {
+          if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(*FA)) {
+            // 'Member' is the anonymous union field and 'AnonUnionMember' is
+            // set to the anonymous union data member used in the initializer
+            // list.
+            Value->setMember(*Field);
+            Value->setAnonUnionMember(*FA);
+            AllToInit.push_back(Value);
+            break;
+          }
+        }
+      }
+      continue;
+    }
+    if (CXXBaseOrMemberInitializer *Value = AllBaseFields.lookup(*Field)) {
+      AllToInit.push_back(Value);
+      continue;
+    }
+    
+    QualType FT = Context.getBaseElementType((*Field)->getType());
+    if (const RecordType* RT = FT->getAs<RecordType>()) {
+      CXXConstructorDecl *Ctor =
+        cast<CXXRecordDecl>(RT->getDecl())->getDefaultConstructor(Context);
+      if (!Ctor && !FT->isDependentType())
+        Fields.push_back(*Field);
+      CXXBaseOrMemberInitializer *Member = 
+      new (Context) CXXBaseOrMemberInitializer((*Field), 0, 0,
+                                         Ctor,
+                                         SourceLocation(),
+                                         SourceLocation());
+      AllToInit.push_back(Member);
+      if (FT.isConstQualified() && (!Ctor || Ctor->isTrivial())) {
+        Diag(Constructor->getLocation(), diag::err_unintialized_member_in_ctor)
+          << Context.getTagDeclType(ClassDecl) << 1 << (*Field)->getDeclName();
+        Diag((*Field)->getLocation(), diag::note_declared_at);
+      }
+    }
+    else if (FT->isReferenceType()) {
+      Diag(Constructor->getLocation(), diag::err_unintialized_member_in_ctor)
+        << Context.getTagDeclType(ClassDecl) << 0 << (*Field)->getDeclName();
+      Diag((*Field)->getLocation(), diag::note_declared_at);
+    }
+    else if (FT.isConstQualified()) {
+      Diag(Constructor->getLocation(), diag::err_unintialized_member_in_ctor)
+        << Context.getTagDeclType(ClassDecl) << 1 << (*Field)->getDeclName();
+      Diag((*Field)->getLocation(), diag::note_declared_at);
+    }
+  }
+  
+  NumInitializers = AllToInit.size();
+  if (NumInitializers > 0) {
+    Constructor->setNumBaseOrMemberInitializers(NumInitializers);
+    CXXBaseOrMemberInitializer **baseOrMemberInitializers =
+      new (Context) CXXBaseOrMemberInitializer*[NumInitializers];
+    
+    Constructor->setBaseOrMemberInitializers(baseOrMemberInitializers);
+    for (unsigned Idx = 0; Idx < NumInitializers; ++Idx)
+      baseOrMemberInitializers[Idx] = AllToInit[Idx];
+  }
+}
+
+void
 Sema::BuildBaseOrMemberInitializers(ASTContext &C,
                                  CXXConstructorDecl *Constructor,
                                  CXXBaseOrMemberInitializer **Initializers,
@@ -898,9 +1072,8 @@
   llvm::SmallVector<CXXBaseSpecifier *, 4>Bases;
   llvm::SmallVector<FieldDecl *, 4>Members;
   
-  Constructor->setBaseOrMemberInitializers(C, 
-                                           Initializers, NumInitializers,
-                                           Bases, Members);
+  setBaseOrMemberInitializers(Constructor, 
+                              Initializers, NumInitializers, Bases, Members);
   for (unsigned int i = 0; i < Bases.size(); i++)
     Diag(Bases[i]->getSourceRange().getBegin(), 
          diag::err_missing_default_constructor) << 0 << Bases[i]->getType();

Added: cfe/trunk/test/SemaCXX/illegal-member-initialization.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/illegal-member-initialization.cpp?rev=80934&view=auto

==============================================================================
--- cfe/trunk/test/SemaCXX/illegal-member-initialization.cpp (added)
+++ cfe/trunk/test/SemaCXX/illegal-member-initialization.cpp Thu Sep  3 14:36:46 2009
@@ -0,0 +1,22 @@
+// RUN: clang-cc -fsyntax-only -verify %s 
+
+struct A {
+   A() : value(), cvalue() { } // expected-error {{cannot initialize the member to null in default constructor because reference member 'value' cannot be null-initialized}} \
+                               // expected-error {{constructor for 'struct A' must explicitly initialize the reference member 'value'}}
+   int &value;	// expected-note{{declared at}}	 {{expected-note{{declared at}}
+   const int cvalue;
+};
+
+struct B {
+};
+
+struct X {
+   X() { }	// expected-error {{constructor for 'struct X' must explicitly initialize the reference member 'value'}} \
+		// expected-error {{constructor for 'struct X' must explicitly initialize the const member 'cvalue'}} \
+		// expected-error {{constructor for 'struct X' must explicitly initialize the reference member 'b'}} \
+		// expected-error {{constructor for 'struct X' must explicitly initialize the const member 'cb'}}
+   int &value; // expected-note{{declared at}}
+   const int cvalue; // expected-note{{declared at}}
+   B& b; // expected-note{{declared at}}
+   const B cb; // expected-note{{declared at}}
+};





More information about the cfe-commits mailing list