PATCH: private ivars

[Adrian Prantl]

are there any remaining issues with this patch that you'd like me to fix?

thanks,
Adrian

On Feb 19, 2013, at 10:35 AM, Adrian Prantl <aprantl at apple.com> wrote:

> 
> On Feb 18, 2013, at 10:47 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
>> If you've disabled caching on the types is there a case when you will have duplicate debug info? What kind of debug info output are you expecting out of the backend for what you're giving it?
> 
> My understanding is that MD nodes are uniqued, is that correct? Based on that assumption we are creating the DIType for the ObjCInterfaceDecl twice, but it will point to the same MDNode. The second time around we are, however, adding a new DW_TAG_member for each additional ivar. 
> 
>> 
>> +  // Do not cache the type if it may be incomplete
>> +  if (maybeIncompleteInterface(Ty))
>> +    return Res;
>> 
>> Maybe you could cache the type and add additional ivars as they're added to the interface?
> 
> Possibly, but keep in mind that the ivar cache is being rebuilt with each extension anyway, this seems to be the least invasive way to do this.
>> 
>> While you're going through some of these and other questions, couple of nits on the patch:
>> 
>> metadata !{i32 786445
>> 
>> the first field is really almost never needed to be checked and will make it more difficult to change later if we decide to change the format.
> fixed.
>> The radar number isn't useful in the testcase, I'd prefer a much longer description of what you're fixing and what the correct behavior should be for the test.
>> 
>> +  // Do not cache the type if it may be incomplete
>> 
>> Complete sentences (ok, just the '.')
> 
> fixed.
> 
>> +  // If the implementation is not yet set, we do not want to mark it
>> +  // as complete. An implementation may declare additional
>> +  // private ivars that we would miss otherwise.
>> +  if (ID->getImplementation() == 0) {
>> +    CompletedTypeCache.erase(QualTy.getAsOpaquePtr());
>> +    //incompleteInterfaces.push_back(std::make_pair(Ty, Unit));
>> +  }
> 
> fixed.
> 
>> -  CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = RealDecl;
>> +  QualType QualTy = QualType(Ty, 0);
>> +  CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl;
>> 
>> Hmm?
> 
> I'm reusing QualTy later on.
> 
>> 
>> +/// Caveat: If you invoke this method before the implementations was
>> +/// the list of ivars will be incomplete. If you call it again after
>> 
> 
> fixed.
> 
> 
> thanks,
> Adrian
> 
> 
> <private-ivars.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits