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

[Fariborz Jahanian]

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