[cfe-commits] r80846 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/CodeGenCXX/constructor-template.cpp

[Douglas Gregor]

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