[cfe-commits] patch for llvm.org/pr8207

Nico Weber thakis at chromium.org
Mon Sep 27 12:49:45 PDT 2010


And now with a more portable test, courtesy of Benjamin Kramer. Only
the CHECK lines changed.

On Sun, Sep 26, 2010 at 3:14 PM, Nico Weber <thakis at chromium.org> wrote:
> Updated the patch with a slightly beefier test case. The actual code
> change is the same as in the previous patch.
>
> Please review.
>
> Nico
>
> On Sat, Sep 25, 2010 at 11:29 PM, Nico Weber <thakis at chromium.org> wrote:
>> Hi,
>>
>> the attached patch fixes llvm.org/pr8207 (and potentially more). The
>> reason the bug happens is the following code in CodeGenModule:
>>
>> LangOptions::VisibilityMode
>> CodeGenModule::getDeclVisibilityMode(const Decl *D) const {
>>  // ...
>>
>>  if (getLangOptions().CPlusPlus) {
>>    // Entities subject to an explicit instantiation declaration get default
>>    // visibility.
>>    if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(D)) {
>>      // ...
>>    } else if (const ClassTemplateSpecializationDecl *ClassSpec
>>                              = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
>>      if (ClassSpec->getSpecializationKind()
>>                                        == TSK_ExplicitInstantiationDeclaration)
>>        return LangOptions::Default;
>>    } else if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(D)) {
>>      if (Record->getTemplateSpecializationKind()
>>                                        == TSK_ExplicitInstantiationDeclaration)
>>        return LangOptions::Default;
>>    } else if (const VarDecl *Var = dyn_cast<VarDecl>(D)) {
>>      // ...
>>    }
>>
>>    // ...
>>  }
>>
>>  // If this decl is contained in a class, it should have the same visibility
>>  // as the parent class.
>>  if (const DeclContext *DC = D->getDeclContext())
>>    if (DC->isRecord())
>>      return getDeclVisibilityMode(cast<Decl>(DC));
>>
>>  return getLangOptions().getVisibilityMode();
>> }
>>
>>
>> When a template specialization declaration is overridden with a
>> template specialization definition, the template specialization kind
>> of inner structs was not updated. Hence, when the function above
>> checked the visibility of the inner struct's function, it would look
>> in the parent class (== the inner struct) because of the code at the
>> end of the function, and then would return LangOptions::Default,
>> because the inner struct would still have specialization kind
>> TSK_ExplicitInstantiationDeclaration.
>>
>> The fix is to correctly re-set the specialization kind of inner
>> structs on the TSK_ExplicitInstantiationDeclaration ->
>> TSK_ExplicitInstantiationDefinition transition on the base class.
>>
>> Nico
>>
>>
>> ps: I'm not sure if the call to MarkVTableUsed is needed, but it's
>> called further up in that file when
>> TSK_ExplicitInstantiationDefinition is set too.
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-visibility.patch
Type: application/octet-stream
Size: 3687 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100927/2eba57d5/attachment.obj>


More information about the cfe-commits mailing list