On Sep 3, 2009, at 7:49 AM, Douglas Gregor wrote: > > 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.
All done in TOT: http://llvm.org/viewvc/llvm-project?view=rev&revision=80950 & http://llvm.org/viewvc/llvm-project?view=rev&revision=80967 - Fariborz > > > - Doug _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
