r189494 - PR16995: Failing to associate static members with their enclosing class

[David Blaikie]

On Wed, Aug 28, 2013 at 12:56 PM, Eric Christopher <echristo at gmail.com> wrote:
>>    for(DeclContext::decl_iterator I = RD->decls_begin(),
>>          E = RD->decls_end(); I != E; ++I) {
>> -    Decl *D = *I;
>> -    if (D->isImplicit())
>> -      continue;
>> -
>> -    if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
>> +    if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(*I)) {
>>        // Reuse the existing member function declaration if it exists
>>        llvm::DenseMap<const FunctionDecl *, llvm::WeakVH>::iterator MI =
>>            SPCache.find(Method->getCanonicalDecl());
>> -      if (MI == SPCache.end())
>> -        EltTys.push_back(CreateCXXMemberFunction(Method, Unit, RecordTy));
>> -      else
>> +      if (MI == SPCache.end()) {
>> +        if (!Method->isImplicit())
>> +          EltTys.push_back(CreateCXXMemberFunction(Method, Unit, RecordTy));
>> +      } else
>>
>
> How did moving the check for implicit into the not-found-in-cache
> branch do anything?

1) emit a declaration for a type (because we don't need a definition -
possibly because it's a dynamic type & we haven't emitted the vtable
yet)
2) emit an implicit member such as an implicit default ctor
3) emit the vtable & consequently the definition of the type, this is
where we hit the above function - rather than appending to the list of
members associated with the declaration & using those for the
definition (because this would cause us to emit members out of source
order), we rebuild the list as above & rely on the fact that the
existing members will be in the SPCache so we'll just wire them up
again.

Except if we skip all implicit members, we'll end up dropping them on
the floor (since we won't even consult the SPCache). So instead, try
the cache, use it if it's there, otherwise /only if it's not implicit/
do we create it.

(& the self-hosting bug I'm reducing a test case for is for the same
thing, but with member function template specializations)

Make sense?