r188642 - Revert "Revert "DebugInfo: Omit debug info for dynamic classes in TUs that do not have the vtable for that class""

[David Blaikie]

On Mon, Aug 19, 2013 at 11:50 AM, Adrian Prantl <aprantl at apple.com> wrote:
> Hi David,
>
> This patch adds several blocks of code without any explanatory comments, which makes it hard to understand what’s going on. Would you mind commenting a bit on what you are doing here?
>
> I’m thinking particularly of
>>
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Sun Aug 18 12:59:12 2013
>> @@ -644,8 +644,24 @@ llvm::DIDescriptor CGDebugInfo::createCo
>>
>>   if (const RecordDecl *RD = dyn_cast<RecordDecl>(Context)) {
>>     if (!RD->isDependentType()) {
>> -      llvm::DIType Ty = getOrCreateLimitedType(
>> -          CGM.getContext().getRecordType(RD)->castAs<RecordType>(), getOrCreateMainFile());
>> +      llvm::DICompositeType T(getTypeOrNull(CGM.getContext().getRecordType(RD)));
>> +      llvm::DICompositeType Ty(getOrCreateLimitedType(
>> +          CGM.getContext().getRecordType(RD)->castAs<RecordType>(),
>> +          getOrCreateMainFile()));
>> +      if (!Ty.getTypeArray().getNumElements()) {
>> +        if (T) {
>> +          llvm::DIArray PrevMem = T.getTypeArray();
>> +          unsigned NumElements = PrevMem.getNumElements();
>> +          if (NumElements == 1 && !PrevMem.getElement(0))
>> +            NumElements = 0;
>> +          SmallVector<llvm::Value *, 16> EltTys;
>> +          EltTys.reserve(NumElements);
>> +          for (unsigned i = 0; i != NumElements; ++i)
>> +            EltTys.push_back(PrevMem.getElement(i));
>> +          llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);
>> +          Ty.setTypeArray(Elements);
>> +        }
>> +      }
>>       return llvm::DIDescriptor(Ty);
>>     }
>>   }
>> @@ -865,7 +881,7 @@ CollectRecordLambdaFields(const CXXRecor
>>   }
>> }

Simplified & commented in r188829.

>
> and
>
>> -/// getStaticDataMemberDeclaration - If D is an out-of-class definition of
>> -/// a static data member of a class, find its corresponding in-class
>> -/// declaration.
>> -llvm::DIDerivedType CGDebugInfo::getStaticDataMemberDeclaration(const VarDecl *D) {
>> -  if (D->isStaticDataMember()) {
>> -    llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator
>> -      MI = StaticDataMemberCache.find(D->getCanonicalDecl());
>> -    if (MI != StaticDataMemberCache.end())
>> -      // Verify the info still exists.
>> -      if (llvm::Value *V = MI->second)
>> -        return llvm::DIDerivedType(cast<llvm::MDNode>(V));
>> +/// If D is an out-of-class definition of a static data member of a class, find
>> +/// its corresponding in-class declaration.
>> +llvm::DIDerivedType
>> +CGDebugInfo::getOrCreateStaticDataMemberDeclarationOrNull(const VarDecl *D) {
>> +  if (!D->isStaticDataMember())
>> +    return llvm::DIDerivedType();
>> +  llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator MI =
>> +      StaticDataMemberCache.find(D->getCanonicalDecl());
>> +  if (MI != StaticDataMemberCache.end()) {
>> +    assert(MI->second && "Static data member declaration should still exist");
>> +    return llvm::DIDerivedType(cast<llvm::MDNode>(MI->second));
>> +  }
>> +  llvm::DICompositeType Ctxt(
>> +      getContextDescriptor(cast<Decl>(D->getDeclContext())));
>> +  llvm::DIDerivedType T = CreateRecordStaticField(D, Ctxt);
>> +  Ctxt.addMember(T);
>> +  return T;

Added a comment here to explain the lazy construction. (r188834).

>> +}
>> +
>> +llvm::DIDerivedType
>> +CGDebugInfo::getOrCreateStaticDataMemberDeclaration(const VarDecl *D,
>> +                                            llvm::DICompositeType Ctxt) {
>> +  llvm::DenseMap<const Decl *, llvm::WeakVH>::iterator MI =
>> +      StaticDataMemberCache.find(D->getCanonicalDecl());
>> +  if (MI != StaticDataMemberCache.end()) {
>> +    assert(MI->second && "Static data member declaration should still exist");
>> +    return llvm::DIDerivedType(cast<llvm::MDNode>(MI->second));
>>   }
>> -  return llvm::DIDerivedType();
>> +  return CreateRecordStaticField(D, Ctxt);
>> }

Rolled this function into the caller with a comment.

>
>
> and
>
>> @@ -3108,7 +3163,7 @@ void CGDebugInfo::EmitGlobalVariable(con
>>     return;
>>   llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
>>       Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init,
>> -      getStaticDataMemberDeclaration(cast<VarDecl>(VD)));
>> +      getOrCreateStaticDataMemberDeclarationOrNull(cast<VarDecl>(VD)));
>>   DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV)));
>> }

Not sure what comment you wanted here - the change was just in the
name of a function to be more descriptive (& reflect a change in
behavior - that this call could lazily construct the declaration
(hence the "OrCreate" clause))

>
> thanks,
> Adrian