From 45cf7e3d2a524e751c2d5c35f2761a8cfbc47fdd Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Fri, 2 Apr 2010 18:24:57 +0000 Subject: Rework our handling of copy construction of temporaries, which was a poor (and wrong) approximation of the actual rules governing when to build a copy and when it can be elided. The correct implementation is actually simpler than the approximation. When we only enumerate constructors as part of initialization (e.g., for direct initialization or when we're copying from a class type or one of its derived classes), we don't create a copy. When we enumerate all conversion functions, we do create a copy. Before, we created some extra copies and missed some others. The new test copy-initialization.cpp shows a case where we missed creating a (required, non-elidable) copy as part of a user-defined conversion, which resulted in a miscompile. This commit also fixes PR6757, where the missing copy made us reject well-formed code in the ternary operator. This commit also cleans up our handling of copy elision in the case where we create an extra copy of a temporary object, which became necessary now that we produce the right copies. The code that seeks to find the temporary object being copied has moved into Expr::getTemporaryObject(); it used to have two different not-quite-the-same implementations, one in Sema and one in CodeGen. Note that we still do not attempt to perform the named return value optimization, so we miss copy elisions for return values and throw expressions. llvm-svn: 100196 --- clang/lib/Sema/SemaDeclCXX.cpp | 42 +++++++++++++++--------------------------- 1 file changed, 15 insertions(+), 27 deletions(-) (limited to 'clang/lib/Sema/SemaDeclCXX.cpp') diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 47df43516c2..39e3739878f 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -3965,33 +3965,21 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType, bool BaseInitialization) { bool Elidable = false; - // C++ [class.copy]p15: - // Whenever a temporary class object is copied using a copy constructor, and - // this object and the copy have the same cv-unqualified type, an - // implementation is permitted to treat the original and the copy as two - // different ways of referring to the same object and not perform a copy at - // all, even if the class copy constructor or destructor have side effects. - - // FIXME: Is this enough? - if (Constructor->isCopyConstructor()) { - Expr *E = ((Expr **)ExprArgs.get())[0]; - if (ImplicitCastExpr *ICE = dyn_cast(E)) - if (ICE->getCastKind() == CastExpr::CK_NoOp) - E = ICE->getSubExpr(); - if (CXXFunctionalCastExpr *FCE = dyn_cast(E)) - E = FCE->getSubExpr(); - while (CXXBindTemporaryExpr *BE = dyn_cast(E)) - E = BE->getSubExpr(); - if (ImplicitCastExpr *ICE = dyn_cast(E)) - if (ICE->getCastKind() == CastExpr::CK_NoOp) - E = ICE->getSubExpr(); - - if (CallExpr *CE = dyn_cast(E)) - Elidable = !CE->getCallReturnType()->isReferenceType(); - else if (isa(E)) - Elidable = true; - else if (isa(E)) - Elidable = true; + // C++0x [class.copy]p34: + // When certain criteria are met, an implementation is allowed to + // omit the copy/move construction of a class object, even if the + // copy/move constructor and/or destructor for the object have + // side effects. [...] + // - when a temporary class object that has not been bound to a + // reference (12.2) would be copied/moved to a class object + // with the same cv-unqualified type, the copy/move operation + // can be omitted by constructing the temporary object + // directly into the target of the omitted copy/move + if (Constructor->isCopyConstructor() && ExprArgs.size() >= 1) { + Expr *SubExpr = ((Expr **)ExprArgs.get())[0]; + Elidable = SubExpr->isTemporaryObject() && + Context.hasSameUnqualifiedType(SubExpr->getType(), + Context.getTypeDeclType(Constructor->getParent())); } return BuildCXXConstructExpr(ConstructLoc, DeclInitType, Constructor, -- cgit v1.2.3