Don't dllimport/export class members with internal linkage (PR23308)

[hans]

Summary

For example, a function taking a parameter with internal linkage will
itself have internal linkage since it cannot be called outside the
translation unit.

[majnemer]

What happens if the export directive is on the function itself instead of the class? Does this deserve a diagnostic? What about the import side?

[hans]

What happens if the export directive is on the function itself instead of the class? Does this deserve a diagnostic? What about the import side?

We diagnose it as an error if put directly on the function.

[thakis]

Looks good to me, thanks.

lib/Sema/SemaDeclCXX.cpp

4807 ↗

(On Diff #24192)

nit: no braces

[hans]

I'll land this now to unbreak the Chromium build. If David or Reid has more comments, we can do follow-up patches.

lib/Sema/SemaDeclCXX.cpp

4807 ↗

(On Diff #24192)

Done.

[rnk]

Lgtm

cfe/trunk/lib/Sema/SemaDeclCXX.cpp

if (MD) {
 
       if (MD->isInlined() &&
           !S.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
         // MinGW does not import or export inline methods.
         continue;
       }
     }
 
+    if (!cast<NamedDecl>(Member)->isExternallyVisible())
+      continue;
+
     if (!getDLLAttr(Member)) {
       auto *NewAttr =
           cast<InheritableAttr>(ClassAttr->clone(S.getASTContext()));
       NewAttr->setInherited(true);
       Member->addAttr(NewAttr);
     }
 
     if (MD && ClassExported) {

cfe/trunk/test/CodeGenCXX/dllexport.cpp

 USEMEMFUNC(ExplicitInstantiationDeclTemplate<int>, f);
 // M32-DAG: {{declare|define available_externally}} x86_thiscallcc void @"\01?f@?$ExplicitInstantiationDeclTemplate@H@@QAEXXZ"
 
 template <typename T> struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate { void f() {} };
 extern template struct ExplicitInstantiationDeclExportedTemplate<int>;
 USEMEMFUNC(ExplicitInstantiationDeclExportedTemplate<int>, f);
 // M32-DAG: {{declare|define available_externally}} x86_thiscallcc void @"\01?f@?$ExplicitInstantiationDeclExportedTemplate@H@@QAEXXZ"
 
+namespace { struct InternalLinkageType {}; }
+struct __declspec(dllexport) PR23308 {
+  void f(InternalLinkageType*);
+};
+void PR23308::f(InternalLinkageType*) {}
+long use(PR23308* p) { p->f(nullptr); }
+// M32-DAG: define internal x86_thiscallcc void @"\01?f@PR23308@@QAEXPAUInternalLinkageType@?A@@@Z"
+
+
 
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
 //===----------------------------------------------------------------------===//
 
 template <typename T> struct ClassTemplate { void func(); };
 template <typename T> void ClassTemplate<T>::func() {}
 template <typename T> struct __declspec(dllexport) ExportedClassTemplate { void func(); };

cfe/trunk/test/SemaCXX/dllexport.cpp

 
 // Warn about explicit instantiation declarations of dllexport classes.
 template <typename T> struct ExplicitInstantiationDeclTemplate {};
 extern template struct __declspec(dllexport) ExplicitInstantiationDeclTemplate<int>; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}} expected-note{{attribute is here}}
 
 template <typename T> struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate {}; // expected-note{{attribute is here}}
 extern template struct ExplicitInstantiationDeclExportedTemplate<int>; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}}
 
+namespace { struct InternalLinkageType {}; }
+struct __declspec(dllexport) PR23308 {
+  void f(InternalLinkageType*);
+};
+void PR23308::f(InternalLinkageType*) {} // No error; we don't try to export f because it has internal linkage.
+
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
 //===----------------------------------------------------------------------===//
 
 template <typename T> class ClassTemplate {};
 template <typename T> class __declspec(dllexport) ExportedClassTemplate {};
 template <typename T> class __declspec(dllimport) ImportedClassTemplate {};