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

Eric Christopher echristo at gmail.com
Wed Aug 28 13:20:42 PDT 2013


On Wed, Aug 28, 2013 at 1:12 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 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.
>

That's... tricky. Could you add this in a comment? Also trying to
figure out how we're emitting the implicit member without emitting the
rest of the type.

> Make sense?

Mostly yeah :)

-eric



More information about the cfe-commits mailing list