On Sep 2, 2009, at 4:02 PM, Fariborz Jahanian wrote: > Author: fjahanian > Date: Wed Sep 2 18:02:57 2009 > New Revision: 80846 > > URL: http://llvm.org/viewvc/llvm-project?rev=80846&view=rev > Log: > Referenced instatiated default constructors > must be defined. Fixed pr4853.
Thanks! Some comments below. > Modified: > cfe/trunk/lib/Sema/SemaDeclCXX.cpp > cfe/trunk/test/CodeGenCXX/constructor-template.cpp > > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=80846&r1=80845&r2=80846&view=diff > > = > = > = > = > = > = > = > = > ====================================================================== > --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Sep 2 18:02:57 2009 > @@ -1003,7 +1003,25 @@ > > if (Constructor->isDependentContext()) > return; > - > + // Mark all constructors used in initialization of class's members > + // as referenced. > + // FIXME. We can do this while building the initializer list. But > + // MarkDeclarationReferenced is not accessible in ASTContext. > + for (CXXConstructorDecl::init_const_iterator B = Constructor- > >init_begin(), > + E = Constructor->init_end(); > + B != E; ++B) { Shouldn't we just move the code from CXXConstructorDecl::setBaseOrMemberInitializers into Sema, so that it can call MarkDeclarationReferenced directly (rather than have to go through the whole list of initializers again later)? Also, I wonder if CXXDestructorDecl::computeBaseOrMembersToDestroy will have the same problem. We'll need to call MarkDeclarationReferenced on each of the destructors it uses. > + CXXBaseOrMemberInitializer *Member = (*B); > + if (!Member->isMemberInitializer()) > + continue; We shouldn't skip out of this loop so soon, since base class initializers will need the same treatment as member initializers. Here's a tweaked version of the test case from PR4826 that is still missing a MarkDeclarationReferenced call: template <typename T> class List { int i; public: List(){ } // List<BinomialNode<int>*>::List() remains undefined. }; template<typename T> class BinomialNode : List<BinomialNode<T>*> { public: BinomialNode(T value) {} // nodes; }; int main() { BinomialNode<int> *node = new BinomialNode<int>(1); } > + FieldDecl *Field = Member->getMember(); > + QualType FT = Context.getBaseElementType(Field->getType()); > + if (const RecordType* RT = FT->getAs<RecordType>()) { > + CXXConstructorDecl *Ctor = > + cast<CXXRecordDecl>(RT->getDecl())->getDefaultConstructor > (Context); > + if (Ctor && !FT->isDependentType()) > + MarkDeclarationReferenced(Ctor->getLocation(), Ctor); The location that we pass to MarkDeclarationReferenced should be the location where the declaration was referenced from. For user-written base or member initializers, that's CXXBaseOrMemberInitializer::IdLoc. For synthesized base or member initializers, we should pick something easily recognizable as being part of the definition, e.g., the '{' that starts the constructor definition. - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
