[cfe-commits] r131520 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/defaulted-ctor-loop.cpp

Sean Hunt scshunt at csclub.uwaterloo.ca
Tue May 17 20:41:58 PDT 2011


Author: coppro
Date: Tue May 17 22:41:58 2011
New Revision: 131520

URL: http://llvm.org/viewvc/llvm-project?rev=131520&view=rev
Log:
Implement an additional fix for infinite recursion of deleted special
member functions by making sure that they're on the record before
checking for deletion.

Also make sure source locations are valid to avoid crashes.

Unfortunately, the declare-all-implicit-members approach is still
required in order to ensure that dependency loops do not result in
incorrectly deleting functions (since they are to be deleted at the
declaration point per the standard).

Fixes PR9917

Added:
    cfe/trunk/test/SemaCXX/defaulted-ctor-loop.cpp
Modified:
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=131520&r1=131519&r2=131520&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue May 17 22:41:58 2011
@@ -3293,6 +3293,8 @@
   if (!LangOpts.CPlusPlus0x)
     return false;
 
+  SourceLocation Loc = CD->getLocation();
+
   // Do access control from the constructor
   ContextRAII CtorContext(*this, CD);
 
@@ -3324,7 +3326,7 @@
     CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
 
@@ -3335,8 +3337,7 @@
     InitializedEntity BaseEntity =
       InitializedEntity::InitializeBase(Context, BI, 0);
     InitializationKind Kind =
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
 
     InitializationSequence InitSeq(*this, BaseEntity, Kind, 0, 0);
 
@@ -3355,7 +3356,7 @@
     CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
 
@@ -3366,8 +3367,7 @@
     InitializedEntity BaseEntity =
       InitializedEntity::InitializeBase(Context, BI, BI);
     InitializationKind Kind =
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
 
     InitializationSequence InitSeq(*this, BaseEntity, Kind, 0, 0);
 
@@ -3400,7 +3400,7 @@
       CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
       if (FieldDtor->isDeleted())
         return true;
-      if (CheckDestructorAccess(SourceLocation(), FieldDtor, PDiag()) !=
+      if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
           AR_accessible)
         return true;
 
@@ -3444,8 +3444,7 @@
     InitializedEntity MemberEntity =
       InitializedEntity::InitializeMember(*FI, 0);
     InitializationKind Kind = 
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
     
     InitializationSequence InitSeq(*this, MemberEntity, Kind, 0, 0);
 
@@ -3467,6 +3466,8 @@
   if (!LangOpts.CPlusPlus0x)
     return false;
 
+  SourceLocation Loc = CD->getLocation();
+
   // Do access control from the constructor
   ContextRAII CtorContext(*this, CD);
 
@@ -3503,7 +3504,7 @@
     CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
 
@@ -3514,15 +3515,13 @@
     InitializedEntity BaseEntity =
       InitializedEntity::InitializeBase(Context, BI, 0);
     InitializationKind Kind =
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
 
     // Construct a fake expression to perform the copy overloading.
     QualType ArgType = BaseType.getUnqualifiedType();
     if (ConstArg)
       ArgType.addConst();
-    Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                              VK_LValue);
+    Expr *Arg = new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue);
 
     InitializationSequence InitSeq(*this, BaseEntity, Kind, &Arg, 1);
 
@@ -3542,7 +3541,7 @@
     CXXDestructorDecl *BaseDtor = LookupDestructor(BaseDecl);
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
 
@@ -3553,15 +3552,13 @@
     InitializedEntity BaseEntity =
       InitializedEntity::InitializeBase(Context, BI, BI);
     InitializationKind Kind =
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
 
     // Construct a fake expression to perform the copy overloading.
     QualType ArgType = BaseType.getUnqualifiedType();
     if (ConstArg)
       ArgType.addConst();
-    Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                              VK_LValue);
+    Expr *Arg = new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue);
 
     InitializationSequence InitSeq(*this, BaseEntity, Kind, &Arg, 1);
 
@@ -3614,7 +3611,7 @@
         CXXDestructorDecl *FieldDtor = LookupDestructor(FieldRecord);
         if (FieldDtor->isDeleted())
           return true;
-        if (CheckDestructorAccess(SourceLocation(), FieldDtor, PDiag()) !=
+        if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
             AR_accessible)
           return true;
       }
@@ -3630,8 +3627,7 @@
     }
 
     InitializationKind Kind = 
-      InitializationKind::CreateDirect(SourceLocation(), SourceLocation(),
-                                       SourceLocation());
+      InitializationKind::CreateDirect(Loc, Loc, Loc);
  
     // Construct a fake expression to perform the copy overloading.
     QualType ArgType = FieldType;
@@ -3639,8 +3635,7 @@
       ArgType = ArgType->getPointeeType();
     else if (ConstArg)
       ArgType.addConst();
-    Expr *Arg = new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                              VK_LValue);
+    Expr *Arg = new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue);
    
     InitializationSequence InitSeq(*this, Entities.back(), Kind, &Arg, 1);
 
@@ -3657,6 +3652,8 @@
   if (!LangOpts.CPlusPlus0x)
     return false;
 
+  SourceLocation Loc = MD->getLocation();
+
   // Do access control from the constructor
   ContextRAII MethodContext(*this, MD);
 
@@ -3672,7 +3669,7 @@
 
   DeclarationName OperatorName =
     Context.DeclarationNames.getCXXOperatorName(OO_Equal);
-  LookupResult R(*this, OperatorName, SourceLocation(), LookupOrdinaryName);
+  LookupResult R(*this, OperatorName, Loc, LookupOrdinaryName);
   R.suppressDiagnostics();
 
   // FIXME: We should put some diagnostic logic right into this function.
@@ -3716,18 +3713,16 @@
     QualType ThisType = BaseType;
     if (ConstArg)
       ArgType.addConst();
-    Expr *Args[] = { new (Context) OpaqueValueExpr(SourceLocation(), ThisType,
-                                                   VK_LValue)
-                   , new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                                   VK_LValue)
+    Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue)
+                   , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue)
                    };
 
-    OverloadCandidateSet OCS((SourceLocation()));
+    OverloadCandidateSet OCS((Loc));
     OverloadCandidateSet::iterator Best;
 
     AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS);
 
-    if (OCS.BestViableFunction(*this, SourceLocation(), Best, false) !=
+    if (OCS.BestViableFunction(*this, Loc, Best, false) !=
         OR_Success)
       return true;
   }
@@ -3763,18 +3758,16 @@
     QualType ThisType = BaseType;
     if (ConstArg)
       ArgType.addConst();
-    Expr *Args[] = { new (Context) OpaqueValueExpr(SourceLocation(), ThisType,
-                                                   VK_LValue)
-                   , new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                                   VK_LValue)
+    Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue)
+                   , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue)
                    };
 
-    OverloadCandidateSet OCS((SourceLocation()));
+    OverloadCandidateSet OCS((Loc));
     OverloadCandidateSet::iterator Best;
 
     AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS);
 
-    if (OCS.BestViableFunction(*this, SourceLocation(), Best, false) !=
+    if (OCS.BestViableFunction(*this, Loc, Best, false) !=
         OR_Success)
       return true;
   }
@@ -3841,18 +3834,16 @@
       QualType ThisType = FieldType;
       if (ConstArg)
         ArgType.addConst();
-      Expr *Args[] = { new (Context) OpaqueValueExpr(SourceLocation(), ThisType,
-                                                     VK_LValue)
-                     , new (Context) OpaqueValueExpr(SourceLocation(), ArgType,
-                                                     VK_LValue)
+      Expr *Args[] = { new (Context) OpaqueValueExpr(Loc, ThisType, VK_LValue)
+                     , new (Context) OpaqueValueExpr(Loc, ArgType, VK_LValue)
                      };
 
-      OverloadCandidateSet OCS((SourceLocation()));
+      OverloadCandidateSet OCS((Loc));
       OverloadCandidateSet::iterator Best;
 
       AddFunctionCandidates(R.asUnresolvedSet(), Args, 2, OCS);
 
-      if (OCS.BestViableFunction(*this, SourceLocation(), Best, false) !=
+      if (OCS.BestViableFunction(*this, Loc, Best, false) !=
           OR_Success)
         return true;
     }
@@ -3867,6 +3858,8 @@
   if (!LangOpts.CPlusPlus0x)
     return false;
 
+  SourceLocation Loc = DD->getLocation();
+
   // Do access control from the destructor
   ContextRAII CtorContext(*this, DD);
 
@@ -3894,7 +3887,7 @@
     //    a destructor that is inaccessible from the defaulted destructor
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
   }
@@ -3910,7 +3903,7 @@
     //    a destructor that is inaccessible from the defaulted destructor
     if (BaseDtor->isDeleted())
       return true;
-    if (CheckDestructorAccess(SourceLocation(), BaseDtor, PDiag()) !=
+    if (CheckDestructorAccess(Loc, BaseDtor, PDiag()) !=
         AR_accessible)
       return true;
   }
@@ -3944,7 +3937,7 @@
         //    inaccessible from the defaulted destructor
         if (FieldDtor->isDeleted())
           return true;
-        if (CheckDestructorAccess(SourceLocation(), FieldDtor, PDiag()) !=
+        if (CheckDestructorAccess(Loc, FieldDtor, PDiag()) !=
           AR_accessible)
         return true;
         
@@ -3960,7 +3953,7 @@
     FunctionDecl *OperatorDelete = 0;
     DeclarationName Name =
       Context.DeclarationNames.getCXXOperatorName(OO_Delete);
-    if (FindDeallocationFunction(SourceLocation(), RD, Name, OperatorDelete,
+    if (FindDeallocationFunction(Loc, RD, Name, OperatorDelete,
           false))
       return true;
   }
@@ -5997,15 +5990,13 @@
   
   // Note that we have declared this constructor.
   ++ASTContext::NumImplicitDefaultConstructorsDeclared;
-
-  // Do not delete this yet if we're in a template
-  if (!ClassDecl->isDependentType() &&
-      ShouldDeleteDefaultConstructor(DefaultCon))
-    DefaultCon->setDeletedAsWritten();
   
   if (Scope *S = getScopeForContext(ClassDecl))
     PushOnScopeChains(DefaultCon, S, false);
   ClassDecl->addDecl(DefaultCon);
+
+  if (ShouldDeleteDefaultConstructor(DefaultCon))
+    DefaultCon->setDeletedAsWritten();
   
   return DefaultCon;
 }
@@ -6706,14 +6697,14 @@
   
   // Note that we have added this copy-assignment operator.
   ++ASTContext::NumImplicitCopyAssignmentOperatorsDeclared;
-  
-  if (ShouldDeleteCopyAssignmentOperator(CopyAssignment))
-    CopyAssignment->setDeletedAsWritten();
 
   if (Scope *S = getScopeForContext(ClassDecl))
     PushOnScopeChains(CopyAssignment, S, false);
   ClassDecl->addDecl(CopyAssignment);
   
+  if (ShouldDeleteCopyAssignmentOperator(CopyAssignment))
+    CopyAssignment->setDeletedAsWritten();
+  
   AddOverriddenMethods(ClassDecl, CopyAssignment);
   return CopyAssignment;
 }
@@ -7187,12 +7178,12 @@
                                                SC_None, 0);
   CopyConstructor->setParams(&FromParam, 1);
 
-  if (ShouldDeleteCopyConstructor(CopyConstructor))
-    CopyConstructor->setDeletedAsWritten();
-
   if (Scope *S = getScopeForContext(ClassDecl))
     PushOnScopeChains(CopyConstructor, S, false);
   ClassDecl->addDecl(CopyConstructor);
+
+  if (ShouldDeleteCopyConstructor(CopyConstructor))
+    CopyConstructor->setDeletedAsWritten();
   
   return CopyConstructor;
 }

Added: cfe/trunk/test/SemaCXX/defaulted-ctor-loop.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/defaulted-ctor-loop.cpp?rev=131520&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/defaulted-ctor-loop.cpp (added)
+++ cfe/trunk/test/SemaCXX/defaulted-ctor-loop.cpp Tue May 17 22:41:58 2011
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++0x %s
+
+// WARNING: This test may recurse infinitely if failing.
+
+struct foo;
+struct bar {
+  bar(foo&);
+};
+struct foo {
+  bar b;
+  foo()
+    : b(b) // expected-warning{{field is uninitialized}}
+  {}
+};





More information about the cfe-commits mailing list