Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6551,6 +6551,8 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
return;
}

TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();

if (Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
!ClassAttr->isInherited()) {
// Diagnose dll attributes on members of class with dll attribute.
Expand All @@ -6561,6 +6563,11 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
if (!MemberAttr || MemberAttr->isInherited() || Member->isInvalidDecl())
continue;

if ((TSK == TSK_ExplicitInstantiationDeclaration ||
TSK == TSK_ExplicitInstantiationDefinition) &&
Member->hasAttr<ExcludeFromExplicitInstantiationAttr>())
continue;

Diag(MemberAttr->getLocation(),
diag::err_attribute_dll_member_of_dll_class)
<< MemberAttr << ClassAttr;
Expand All @@ -6583,8 +6590,6 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
!ClassExported &&
cast<DLLImportAttr>(ClassAttr)->wasPropagatedToBaseTemplate();

TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();

// Ignore explicit dllexport on explicit class template instantiation
// declarations, except in MinGW mode.
if (ClassExported && !ClassAttr->isInherited() &&
Expand All @@ -6601,6 +6606,11 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
// seem to be true in practice?

for (Decl *Member : Class->decls()) {
if ((TSK == TSK_ExplicitInstantiationDeclaration ||
TSK == TSK_ExplicitInstantiationDefinition) &&
Member->hasAttr<ExcludeFromExplicitInstantiationAttr>())
continue;

VarDecl *VD = dyn_cast<VarDecl>(Member);
CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Member);

Expand Down Expand Up @@ -19109,7 +19119,17 @@ bool Sema::DefineUsedVTables() {
}
}

if (IsExplicitInstantiationDeclaration)
if (IsExplicitInstantiationDeclaration &&
llvm::none_of(Class->decls(), [](Decl *decl) {
// If the class has a virtual member function declared with
// `__attribute__((exclude_from_explicit_instantiation))`, the
// explicit instantiation declaration shouldn't suppress emitting
// the vtable to ensure that the excluded member function is
// accessible through the vtable.
auto *Method = dyn_cast<CXXMethodDecl>(decl);
return Method && Method->isVirtual() &&
Method->hasAttr<ExcludeFromExplicitInstantiationAttr>();
}))
Comment on lines +19123 to +19132
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a style issue, I think it would be easier to read if we put this in a variable, like:

bool HasExcludeFromExplicitInstantiation = llvm::any_of(... hasAttr<ExcludeFromExlicitInstantiationAttr>() ...

Also, I'm not sure that it matters whether the method with the attribute is virtual or not. I think it would be enough to check that any method has the attribute -- because then the motivation about "suppress the vtable; it will live with the explicit instantiation definition" may not hold.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could break this out into a separate PR as well, since it seems somewhat separate from the other issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a style issue, I think it would be easier to read if we put this in a variable

Sure.

Also, I'm not sure that it matters whether the method with the attribute is virtual or not. I think it would be enough to check that any method has the attribute -- because then the motivation about "suppress the vtable; it will live with the explicit instantiation definition" may not hold.

I don't understand why non-virtual functions matter here. If no virtual methods are excluded, then no special handling is needed to enforce emitting the definition. The vtable itself is emitted by another procedure anyway (I still can't find the procedure, though).

I wonder if we could break this out into a separate PR as well, since it seems somewhat separate from the other issue.

No, this should be a part of the PR because this is the cause of reverting #65961.

DefineVTable = false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// RUN: %clang_cc1 -triple x86_64-win32 -fms-extensions -emit-llvm -o - %s | \
// RUN: FileCheck %s --check-prefixes=MSC --implicit-check-not=to_be_ --implicit-check-not=dllexport
// RUN: %clang_cc1 -triple x86_64-mingw -emit-llvm -o - %s | \
// RUN: FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_ --implicit-check-not=dllexport
// RUN: %clang_cc1 -triple x86_64-cygwin -emit-llvm -o - %s | \
// RUN: FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_ --implicit-check-not=dllexport

// Test that __declspec(dllexport) doesn't instantiate entities marked with
// the exclude_from_explicit_instantiation attribute unless marked as dllexport explicitly.

// MSC: ModuleID = {{.*}}exclude_from_dllexport.cpp
// MSC: source_filename = {{.*}}exclude_from_dllexport.cpp
// GNU: ModuleID = {{.*}}exclude_from_dllexport.cpp
// GNU: source_filename = {{.*}}exclude_from_dllexport.cpp
Comment on lines +11 to +14
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks seem redundant. (Same in the other file.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just to silence --implicit-check=dllexport.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the --implicit'check-not's are a little bit confusing here. I think the test may be easier to read with explicit checks, and then you could drop these lines.

If not, I think we need some comments explaining the check-not's.


#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))

template <class T>
struct C {
// This will be instantiated explicitly as an exported function because it
// inherits dllexport from the class instantiation.
void to_be_exported();

// This will be instantiated implicitly as an exported function because it is
// marked as dllexport explicitly.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION __declspec(dllexport) void to_be_exported_explicitly();

// This will be instantiated implicitly but won't be exported.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_exported();

// This won't be instantiated.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_instantiated();
};

template <class T> void C<T>::to_be_exported() {}
template <class T> void C<T>::to_be_exported_explicitly() {}
template <class T> void C<T>::not_to_be_exported() {}
template <class T> void C<T>::not_to_be_instantiated() {}

// Attach the attribute to class template declaration instead of instantiation declaration.
template <class T>
struct __declspec(dllexport) D {
// This will be exported if and only if no explicit instantiations are provided.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION void to_be_exported_iff_no_explicit_instantiation();
};

template <class T> void D<T>::to_be_exported_iff_no_explicit_instantiation() {}

// Interaction with VTables.
template <class T>
struct E {
// This will be instanciated by the explicit template instantiation definition.
virtual void to_be_exported();

// This will be instantiated by the VTable definition, regardless of
// `exclude_from_explicit_instantiation`.
// The dllexport attribute won't be inherited.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION virtual void to_be_instantiated();

// This too, but will be exported by the member attribute.
EXCLUDE_FROM_EXPLICIT_INSTANTIATION __declspec(dllexport) virtual void to_be_exported_explicitly();
};

template <class T> void E<T>::to_be_exported() {}
template <class T> void E<T>::to_be_instantiated() {}
template <class T> void E<T>::to_be_exported_explicitly() {}

// MSC: $"?to_be_exported@?$C@H@@QEAAXXZ" = comdat any
// MSC: $"?to_be_exported@?$E@H@@UEAAXXZ" = comdat any
// MSC: $"?to_be_exported@?$E@I@@UEAAXXZ" = comdat any
// MSC: $"?to_be_exported_explicitly@?$C@H@@QEAAXXZ" = comdat any
// MSC: $"?not_to_be_exported@?$C@H@@QEAAXXZ" = comdat any
// MSC: $"?to_be_exported_iff_no_explicit_instantiation@?$D@H@@QEAAXXZ" = comdat any
// MSC: $"?to_be_exported_iff_no_explicit_instantiation@?$D@I@@QEAAXXZ" = comdat any
// MSC: $"?to_be_instantiated@?$E@H@@UEAAXXZ" = comdat any
// MSC: $"?to_be_exported_explicitly@?$E@H@@UEAAXXZ" = comdat any
// MSC: $"?to_be_instantiated@?$E@I@@UEAAXXZ" = comdat any
// MSC: $"?to_be_exported_explicitly@?$E@I@@UEAAXXZ" = comdat any
// GNU: $_ZN1CIiE14to_be_exportedEv = comdat any
// GNU: $_ZN1EIiE14to_be_exportedEv = comdat any
// GNU: $_ZN1EIjE14to_be_exportedEv = comdat any
// GNU: $_ZN1CIiE25to_be_exported_explicitlyEv = comdat any
// GNU: $_ZN1CIiE18not_to_be_exportedEv = comdat any
// GNU: $_ZN1DIiE44to_be_exported_iff_no_explicit_instantiationEv = comdat any
// GNU: $_ZN1DIjE44to_be_exported_iff_no_explicit_instantiationEv = comdat any
// GNU: $_ZN1EIiE18to_be_instantiatedEv = comdat any
// GNU: $_ZN1EIiE25to_be_exported_explicitlyEv = comdat any
// GNU: $_ZN1EIjE18to_be_instantiatedEv = comdat any
// GNU: $_ZN1EIjE25to_be_exported_explicitlyEv = comdat any

// MSC: @0 = private unnamed_addr constant {{.*}}, comdat($"??_7?$E@H@@6B@")
// MSC: @1 = private unnamed_addr constant {{.*}}, comdat($"??_7?$E@I@@6B@")
// MSC: @"??_7?$E@H@@6B@" = dllexport unnamed_addr
// MSC: @"??_7?$E@I@@6B@" = unnamed_addr
// GNU:@_ZTV1EIiE = weak_odr dso_local dllexport unnamed_addr constant {{.*}}, comdat
// GNU:@_ZTV1EIjE = weak_odr dso_local unnamed_addr constant {{.*}}, comdat

// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$C@H@@QEAAAEAU0@AEBU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$C@H@@QEAAAEAU0@$$QEAU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} void @"?to_be_exported@?$C@H@@QEAAXXZ"
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1CIiEaSERKS0_
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1CIiEaSEOS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1CIiE14to_be_exportedEv
template struct __declspec(dllexport) C<int>;

// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$D@H@@QEAAAEAU0@AEBU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$D@H@@QEAAAEAU0@$$QEAU0@@Z"
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1DIiEaSERKS0_
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1DIiEaSEOS0_
template struct D<int>; // No dllexport here.
// Don't provide explicit instantiation for D<unsigned>.

// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$E@H@@QEAAAEAU0@AEBU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$E@H@@QEAAAEAU0@$$QEAU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??0?$E@H@@QEAA@XZ"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??0?$E@H@@QEAA@AEBU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??0?$E@H@@QEAA@$$QEAU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} void @"?to_be_exported@?$E@H@@UEAAXXZ"
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1EIiEaSERKS0_
// GNU: define weak_odr dso_local dllexport{{.*}} ptr @_ZN1EIiEaSEOS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC2Ev
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC1Ev
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC2ERKS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC1ERKS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC2EOS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiEC1EOS0_
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1EIiE14to_be_exportedEv
template struct __declspec(dllexport) E<int>;

// MSC: define weak_odr dso_local{{.*}} void @"?to_be_exported@?$E@I@@UEAAXXZ"
// GNU: define weak_odr dso_local{{.*}} void @_ZN1EIjE14to_be_exportedEv
template struct E<unsigned int>;

// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$D@I@@QEAAAEAU0@AEBU0@@Z"
// MSC: define weak_odr dso_local dllexport{{.*}} ptr @"??4?$D@I@@QEAAAEAU0@$$QEAU0@@Z"

void use() {
C<int> c;

// MSC: call void @"?to_be_exported_explicitly@?$C@H@@QEAAXXZ"
// GNU: call void @_ZN1CIiE25to_be_exported_explicitlyEv
c.to_be_exported_explicitly(); // implicitly instantiated here

// MSC: call void @"?not_to_be_exported@?$C@H@@QEAAXXZ"
// GNU: call void @_ZN1CIiE18not_to_be_exportedEv
c.not_to_be_exported(); // implicitly instantiated here

D<int> di;

// MSC: call void @"?to_be_exported_iff_no_explicit_instantiation@?$D@H@@QEAAXXZ"
// GNU: call void @_ZN1DIiE44to_be_exported_iff_no_explicit_instantiationEv
di.to_be_exported_iff_no_explicit_instantiation(); // implicitly instantiated here

D<unsigned> dj;

// MSC: call void @"?to_be_exported_iff_no_explicit_instantiation@?$D@I@@QEAAXXZ"
// GNU: call void @_ZN1DIjE44to_be_exported_iff_no_explicit_instantiationEv
dj.to_be_exported_iff_no_explicit_instantiation(); // implicitly instantiated here
}

// MSC: define weak_odr dso_local dllexport{{.*}} void @"?to_be_exported_explicitly@?$C@H@@QEAAXXZ"
// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1CIiE25to_be_exported_explicitlyEv

// MSC: define linkonce_odr dso_local void @"?not_to_be_exported@?$C@H@@QEAAXXZ"
// GNU: define linkonce_odr dso_local void @_ZN1CIiE18not_to_be_exportedEv

// MSC: define linkonce_odr dso_local void @"?to_be_exported_iff_no_explicit_instantiation@?$D@H@@QEAAXXZ"
// MSC: define weak_odr dso_local dllexport void @"?to_be_exported_iff_no_explicit_instantiation@?$D@I@@QEAAXXZ"
// GNU: define linkonce_odr dso_local void @_ZN1DIiE44to_be_exported_iff_no_explicit_instantiationEv
// GNU: define weak_odr dso_local dllexport void @_ZN1DIjE44to_be_exported_iff_no_explicit_instantiationEv

// MSC: define linkonce_odr dso_local void @"?to_be_instantiated@?$E@H@@UEAAXXZ"
// MSC: define weak_odr dso_local dllexport void @"?to_be_exported_explicitly@?$E@H@@UEAAXXZ"
// GNU: define linkonce_odr dso_local void @_ZN1EIiE18to_be_instantiatedEv
// GNU: define weak_odr dso_local dllexport void @_ZN1EIiE25to_be_exported_explicitlyEv

// MSC: define linkonce_odr dso_local void @"?to_be_instantiated@?$E@I@@UEAAXXZ"
// MSC: define weak_odr dso_local dllexport void @"?to_be_exported_explicitly@?$E@I@@UEAAXXZ"
// GNU: define linkonce_odr dso_local void @_ZN1EIjE18to_be_instantiatedEv
// GNU: define weak_odr dso_local dllexport void @_ZN1EIjE25to_be_exported_explicitlyEv
Loading