[cfe-commits] r105487 - in /cfe/trunk: lib/CodeGen/CGBlocks.cpp lib/Sema/SemaExpr.cpp test/CodeGenCXX/copy-in-cplus-object.cpp

[Douglas Gregor]

On Jun 4, 2010, at 2:35 PM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Fri Jun  4 16:35:44 2010
> New Revision: 105487
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=105487&view=rev
> Log:
> Build AST for copy-construction of copied-in
> class object in blocks and carry it to IRGen.
> 
> 
> Modified:
>    cfe/trunk/lib/CodeGen/CGBlocks.cpp
>    cfe/trunk/lib/Sema/SemaExpr.cpp
>    cfe/trunk/test/CodeGenCXX/copy-in-cplus-object.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=105487&r1=105486&r2=105487&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Fri Jun  4 16:35:44 2010
> @@ -361,33 +361,21 @@
>           Builder.CreateStore(Loc, Addr);
>           continue;
>         } else {
> -          E = new (getContext()) DeclRefExpr(const_cast<ValueDecl*>(VD),
> +            if (BDRE->getCopyConstructorExpr())
> +              E = BDRE->getCopyConstructorExpr();
> +            else {
> +              E = new (getContext()) DeclRefExpr(const_cast<ValueDecl*>(VD),
>                                             VD->getType().getNonReferenceType(),
>                                             SourceLocation());
> -          if (getContext().getLangOptions().CPlusPlus) {
> -            if (VD->getType()->isReferenceType()) {
> -              E = new (getContext())
> -                UnaryOperator(const_cast<Expr*>(E), UnaryOperator::AddrOf,
> -                              getContext().getPointerType(E->getType()),
> -                              SourceLocation());
> -            } 
> -            else {
> -              QualType T = E->getType();
> -              if (const RecordType *RT = T->getAs<RecordType>()) {
> -                CXXRecordDecl *Record = cast<CXXRecordDecl>(RT->getDecl());
> -                if (!Record->hasTrivialCopyConstructor()) {
> -                  CXXConstructorDecl *D = Record->getCopyConstructor(getContext(),
> -                                                                     0);
> -                  Expr *Arg = const_cast<Expr*>(E);
> -                  E = CXXConstructExpr::Create(getContext(), T, D->getLocation(), 
> -                                               D, false, &Arg, 1, false,
> -                                               CXXConstructExpr::CK_Complete);
> -                }
> -              }
> +              if (VD->getType()->isReferenceType()) {
> +                E = new (getContext())
> +                    UnaryOperator(const_cast<Expr*>(E), UnaryOperator::AddrOf,
> +                                getContext().getPointerType(E->getType()),
> +                                SourceLocation());
> +              } 
>             }
>           }
>         }
> -      }

Okay.

>       if (BDRE->isByRef()) {
>         E = new (getContext())
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=105487&r1=105486&r2=105487&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Jun  4 16:35:44 2010
> @@ -1746,8 +1746,28 @@
>     // Variable will be bound by-copy, make it const within the closure.
> 
>     ExprTy.addConst();
> -    return Owned(new (Context) BlockDeclRefExpr(VD, ExprTy, Loc, false,
> -                                                constAdded));
> +    BlockDeclRefExpr *BDRE = new (Context) BlockDeclRefExpr(VD, 
> +                                                            ExprTy, Loc, false,
> +                                                            constAdded);
> +    QualType T = VD->getType();
> +    if (getLangOptions().CPlusPlus && !T->isDependentType() &&
> +        !T->isReferenceType()) {
> +      Expr *E = new (Context) 
> +                  DeclRefExpr(const_cast<ValueDecl*>(BDRE->getDecl()), T,
> +                                         SourceLocation());
> +      
> +      OwningExprResult Res = PerformCopyInitialization(
> +                      InitializedEntity::InitializeResult(SourceLocation(), 
> +                                                          T, false),
> +                      SourceLocation(),
> +                      Owned(E));

The use of InitializeResult is a little strange, since that's meant for function return values. None of the current InitializedEntities really fits this case. Perhaps we should add a new initialized entity for captured block variables? It could greatly improve diagnostics for this case.

> +      if (!Res.isInvalid()) {
> +        Expr *Init = Res.takeAs<Expr>();
> +        if (isa<CXXConstructExpr>(Init))
> +          BDRE->setCopyConstructorExpr(Init);
> +      }

This code should call MaybeCreateCXXExprWithTemporaries to destroy any temporaries created as part of the call to the copy constructor, e.g., for a copy constructor such as:

	X(const X&, const std::string &Silly = "silly");

where we need to destroy the temporary std::string. 

	- Doug