On May 1, 2011, at 12:04 AM, Sean Hunt wrote: > Modified: cfe/trunk/include/clang/AST/DeclCXX.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=130642&r1=130641&r2=130642&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/DeclCXX.h (original) > +++ cfe/trunk/include/clang/AST/DeclCXX.h Sun May 1 02:04:31 2011 > @@ -1608,6 +1608,23 @@ > void setCtorInitializers(CXXCtorInitializer ** initializers) { > CtorInitializers = initializers; > } > + > + /// isDelegatingConstructor - Whether this constructor is a > + /// delegating constructor > + bool isDelegatingConstructor() const { > + return (getNumCtorInitializers() == 1) && > + CtorInitializers[0]->isDelegatingInitializer(); > + } > + > + /// getTargetConstructor - When this constructor delegates to > + /// another, retrieve the target > + CXXConstructorDecl *getTargetConstructor() const { > + if (isDelegatingConstructor()) > + return CtorInitializers[0]->getTargetConstructor(); > + else > + return 0; > + } > +
My preference would be for this method to assert(isDelegatingConstructor()). Also, I think the name ought to have 'delegate' in there somewhere, maybe 'getDelegateTargetConstructor()'. > Modified: cfe/trunk/lib/CodeGen/CGClass.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=130642&r1=130641&r2=130642&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGClass.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGClass.cpp Sun May 1 02:04:31 2011 > @@ -658,6 +658,10 @@ > if (Ctor->getType()->getAs<FunctionProtoType>()->isVariadic()) > return false; > > + // FIXME: Decide if we can do a delegation of a delegating constructor. > + if (Ctor->isDelegatingConstructor()) > + return false; > + Complete-to-base delegation is still fine for delegating constructors if there are no virtual bases, because by definition the complete and base variants of the target constructor are equivalent. > @@ -721,8 +728,10 @@ > > if (Member->isBaseInitializer()) > EmitBaseInitializer(*this, ClassDecl, Member, CtorType); > - else > + else if (Member->isAnyMemberInitializer()) > MemberInitializers.push_back(Member); > + else > + llvm_unreachable("Delegating initializer on non-delegating > constructor"); > } Better to just assert(Member->isAnyMemberInitializer()); > +void > +CodeGenFunction::EmitDelegatingCXXConstructorCall(const CXXConstructorDecl > *Ctor, > + const FunctionArgList > &Args) { > + assert(Ctor->isDelegatingConstructor()); > + > + llvm::Value *ThisPtr = LoadCXXThis(); > + > + AggValueSlot AggSlot = AggValueSlot::forAddr(ThisPtr, false, /*Lifetime*/ > true); > + > + EmitAggExpr(Ctor->init_begin()[0]->getInit(), AggSlot); > +} This is clever. After the delegate call succeeds, though, you do need to push a cleanup to call the dtor if an exception is thrown in the body of the delegating constructor. > Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=130642&r1=130641&r2=130642&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sun May 1 02:04:31 2011 > @@ -404,9 +404,16 @@ > E->arg_begin(), E->arg_end()); > } > else { > - CXXCtorType Type = > - (E->getConstructionKind() == CXXConstructExpr::CK_Complete) > - ? Ctor_Complete : Ctor_Base; > + CXXCtorType Type; > + CXXConstructExpr::ConstructionKind K = E->getConstructionKind(); > + if (K == CXXConstructExpr::CK_Delegating) { > + // We should be emitting a constructor; GlobalDecl will assert this > + Type = CurGD.getCtorType(); > + } else { > + Type = (E->getConstructionKind() == CXXConstructExpr::CK_Complete) > + ? Ctor_Complete : Ctor_Base; > + } Since this is already inside an if/else, go ahead and break up this ternary operator, or even consider using a switch over the construction kind. > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=130642&r1=130641&r2=130642&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sun May 1 02:04:31 2011 > @@ -2068,6 +2068,30 @@ > > return false; > } > + > +bool > +Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor, > + CXXCtorInitializer *Initializer) { > + Constructor->setNumCtorInitializers(1); > + CXXCtorInitializer **initializer = > + new (Context) CXXCtorInitializer*[1]; > + memcpy(initializer, &Initializer, sizeof (CXXCtorInitializer*)); > + Constructor->setCtorInitializers(initializer); > + > + // FIXME: This doesn't catch indirect loops yet > + CXXConstructorDecl *Target = Initializer->getTargetConstructor(); > + while (Target) { > + if (Target == Constructor) { > + Diag(Initializer->getSourceLocation(), diag::err_delegating_ctor_loop) > + << Constructor; > + return true; > + } > + Target = Target->getTargetConstructor(); > + } > + > + return false; > +} Is this FIXME still correct? It certainly looks like it detects loops of arbitrary length. However, it also looks like it can infinite-loop if there's a loop not involving the current constructor, e.g. MyClass() : MyClass(0) {} MyClass(int x) : MyClass() { this->x = x; } // should get an error here, but then... MyClass(int x, int y) : MyClass(x) { this->y = y; } // this will infinite loop. John.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
