From: John McCall <rjmccall@apple.com>
Date: Wed, 19 Jan 2011 11:48:09 +0000 (+0000)
Subject: When building the copy expression for a __block variable, make sure
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2998d6b0e6d5476c473f68012a851fe4a0f609c8;p=clang

When building the copy expression for a __block variable, make sure
there's a respectable point of instantiation.  Also, make sure we do
this operation even when instantiating a dependently-typed variable.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@123818 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 1f383e6ca8..a130df6f7a 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -716,6 +716,7 @@ public:
                                      bool &Redeclaration);
   void CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous,
                                 bool &Redeclaration);
+  void CheckCompleteVariableDeclaration(VarDecl *var);
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      QualType R, TypeSourceInfo *TInfo,
                                      LookupResult &Previous,
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index a37a97343f..6bbbd541c2 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -3056,26 +3056,6 @@ Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     if (isExplicitSpecialization && !NewVD->isInvalidDecl() &&
         CheckMemberSpecialization(NewVD, Previous))
       NewVD->setInvalidDecl();
-    // For variables declared as __block which require copy construction,
-    // must capture copy initialization expression here.
-    if (!NewVD->isInvalidDecl() && NewVD->hasAttr<BlocksAttr>()) {
-      QualType T = NewVD->getType();
-      if (!T->isDependentType() && !T->isReferenceType() &&
-          T->getAs<RecordType>() && !T->isUnionType()) {
-        Expr *E = new (Context) DeclRefExpr(NewVD, T,
-                                            VK_LValue, SourceLocation());
-        ExprResult Res = PerformCopyInitialization(
-                InitializedEntity::InitializeBlock(NewVD->getLocation(), 
-                                                   T, false),
-                SourceLocation(),
-                Owned(E));
-        if (!Res.isInvalid()) {
-          Res = MaybeCreateExprWithCleanups(Res);
-          Expr *Init = Res.takeAs<Expr>();
-          Context.setBlockVarCopyInits(NewVD, Init);
-        }
-      }
-    }
   }
   
   // attributes declared post-definition are currently ignored
@@ -4712,24 +4692,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   // Attach the initializer to the decl.
   VDecl->setInit(Init);
 
-  if (getLangOptions().CPlusPlus) {
-    if (!VDecl->isInvalidDecl() &&
-        !VDecl->getDeclContext()->isDependentContext() &&
-        VDecl->hasGlobalStorage() && !VDecl->isStaticLocal() &&
-        !Init->isConstantInitializer(Context,
-                                     VDecl->getType()->isReferenceType()))
-      Diag(VDecl->getLocation(), diag::warn_global_constructor)
-        << Init->getSourceRange();
-
-    // Make sure we mark the destructor as used if necessary.
-    QualType InitType = VDecl->getType();
-    while (const ArrayType *Array = Context.getAsArrayType(InitType))
-      InitType = Context.getBaseElementType(Array);
-    if (const RecordType *Record = InitType->getAs<RecordType>())
-      FinalizeVarWithDestructor(VDecl, Record);
-  }
-
-  return;
+  CheckCompleteVariableDeclaration(VDecl);
 }
 
 /// ActOnInitializerError - Given that there was an error parsing an
@@ -4923,20 +4886,60 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl,
                                         MultiExprArg(*this, 0, 0));
       if (Init.isInvalid())
         Var->setInvalidDecl();
-      else if (Init.get()) {
+      else if (Init.get())
         Var->setInit(MaybeCreateExprWithCleanups(Init.get()));
+    }
+
+    CheckCompleteVariableDeclaration(Var);
+  }
+}
 
-        if (getLangOptions().CPlusPlus && !Var->isInvalidDecl() && 
-            Var->hasGlobalStorage() && !Var->isStaticLocal() &&
-            !Var->getDeclContext()->isDependentContext() &&
-            !Var->getInit()->isConstantInitializer(Context, false))
-          Diag(Var->getLocation(), diag::warn_global_constructor);
+void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
+  if (var->isInvalidDecl()) return;
+
+  // All the following checks are C++ only.
+  if (!getLangOptions().CPlusPlus) return;
+
+  QualType baseType = Context.getBaseElementType(var->getType());
+  if (baseType->isDependentType()) return;
+
+  // __block variables might require us to capture a copy-initializer.
+  if (var->hasAttr<BlocksAttr>()) {
+    // It's currently invalid to ever have a __block variable with an
+    // array type; should we diagnose that here?
+
+    // Regardless, we don't want to ignore array nesting when
+    // constructing this copy.
+    QualType type = var->getType();
+
+    if (type->isStructureOrClassType()) {
+      SourceLocation poi = var->getLocation();
+      Expr *varRef = new (Context) DeclRefExpr(var, type, VK_LValue, poi);
+      ExprResult result =
+        PerformCopyInitialization(
+                        InitializedEntity::InitializeBlock(poi, type, false),
+                                  poi, Owned(varRef));
+      if (!result.isInvalid()) {
+        result = MaybeCreateExprWithCleanups(result);
+        Expr *init = result.takeAs<Expr>();
+        Context.setBlockVarCopyInits(var, init);
       }
     }
-
-    if (!Var->isInvalidDecl() && getLangOptions().CPlusPlus && Record)
-      FinalizeVarWithDestructor(Var, Record);
   }
+
+  // Check for global constructors.
+  if (!var->getDeclContext()->isDependentContext() &&
+      var->hasGlobalStorage() &&
+      !var->isStaticLocal() &&
+      var->getInit() &&
+      !var->getInit()->isConstantInitializer(Context,
+                                             baseType->isReferenceType()))
+    Diag(var->getLocation(), diag::warn_global_constructor)
+      << var->getInit()->getSourceRange();
+
+  // Require the destructor.
+  if (const RecordType *recordType = baseType->getAs<RecordType>())
+    FinalizeVarWithDestructor(var, recordType);
 }
 
 Sema::DeclGroupPtrTy
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index fd8d1e1e82..b2749bf6ce 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5641,16 +5641,7 @@ void Sema::AddCXXDirectInitializerToDecl(Decl *RealDecl,
   VDecl->setInit(Result.takeAs<Expr>());
   VDecl->setCXXDirectInitializer(true);
 
-    if (!VDecl->isInvalidDecl() &&
-        !VDecl->getDeclContext()->isDependentContext() &&
-        VDecl->hasGlobalStorage() && !VDecl->isStaticLocal() &&
-        !VDecl->getInit()->isConstantInitializer(Context,
-                                        VDecl->getType()->isReferenceType()))
-      Diag(VDecl->getLocation(), diag::warn_global_constructor)
-        << VDecl->getInit()->getSourceRange();
-
-  if (const RecordType *Record = VDecl->getType()->getAs<RecordType>())
-    FinalizeVarWithDestructor(VDecl, Record);
+  CheckCompleteVariableDeclaration(VDecl);
 }
 
 /// \brief Given a constructor and the set of arguments provided for the
diff --git a/test/SemaObjCXX/blocks.mm b/test/SemaObjCXX/blocks.mm
index f595863815..d3f4d9823c 100644
--- a/test/SemaObjCXX/blocks.mm
+++ b/test/SemaObjCXX/blocks.mm
@@ -74,3 +74,23 @@ namespace N1 {
       });
   }
 }
+
+// Make sure we successfully instantiate the copy constructor of a
+// __block variable's type.
+namespace N2 {
+  template <int n> struct A {
+    A() {}
+    A(const A &other) {
+      int invalid[-n]; // expected-error 2 {{array with a negative size}}
+    }
+  };
+
+  void test1() {
+    __block A<1> x; // expected-note {{requested here}}
+  }
+
+  template <int n> void test2() {
+    __block A<n> x; // expected-note {{requested here}}
+  }
+  template void test2<2>();
+}