[ms][dll] #27212: Generating of implicit special members should take into account MSVC compatibility version

[avt77]

Summary

Clang creates implicit move constructor/assign operator in all cases if there is std=c++11. But MSVC supports such generation starting from version 1900 only. As result we have some binary incompatibility.

[rnk]

As mentioned twice in https://llvm.org/bugs/show_bug.cgi?id=27212, I don't think this is the right direction. To my knowledge, this only causes an ABI break when importing a class. I think we should change Sema::checkClassLevelDLLAttribute to not apply dllimport to implicit move special members instead.

[avt77]

It seems it will be even shorter if we do it via Sema::checkClassLevelDLLAttribute. Tnx.

[avt77]

Now it's really a micro-patch: please review.

[rnk]

lib/Sema/SemaDeclCXX.cpp

4813

Oh, so we were already doing this check. I don't see what's wrong with our current behavior, though. We export a few more symbols than MSVC 2013, but there's no ABI problem with that.

[rnk]

lib/Sema/SemaDeclCXX.cpp

4816

The move constructor part of this is definitely a good fix though.

[avt77]

lib/Sema/SemaDeclCXX.cpp

4813

Yes, it's a question about binary compatibility only

4816

And what's the decision? Could I commit the patch?

[rnk]

lgtm

I've been trying to say that ABI compatibility is not the same thing as generating the same set of exported symbols, but it really doesn't matter. If MSVC 2013 and 2015 are not ABI compatible in this corner case, there is no need for clang to bend over backward to provide a marginally better experience. It sounds like it makes your life easier if we can always produce the exact same set of exports as the MSVC version we are targeting.

[DmitryPolukhin]

Committed as rL269400

lib/Sema/SemaDeclCXX.cpp

void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
   }
 
   if (Class->getDescribedClassTemplate())
     // Don't inherit dll attribute until the template is instantiated.
     return;
 
   // The class is either imported or exported.
   const bool ClassExported = ClassAttr->getKind() == attr::DLLExport;
-  const bool ClassImported = !ClassExported;
 
   TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();
 
   // Ignore explicit dllexport on explicit class template instantiation declarations.
   if (ClassExported && !ClassAttr->isInherited() &&
       TSK == TSK_ExplicitInstantiationDeclaration) {
     Class->dropAttr<DLLExportAttr>();
     return;
if (MD) {
       if (MD->isDeleted())
         continue;
 
       if (MD->isInlined()) {
         // MinGW does not import or export inline methods.
         if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
           continue;
 
-        // MSVC versions before 2015 don't export the move assignment operators,
-        // so don't attempt to import them if we have a definition.
-        if (ClassImported && MD->isMoveAssignmentOperator() &&
+        // MSVC versions before 2015 don't export the move assignment operators
+        // and move constructor, so don't attempt to import/export them if
+        // we have a definition.
+        auto *CXXC = dyn_cast<CXXConstructorDecl>(MD);
+        if ((MD->isMoveAssignmentOperator() ||
+             (CXXC && CXXC->isMoveConstructor())) &&
             !getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015))
           continue;
       }
     }
 
     if (!cast<NamedDecl>(Member)->isExternallyVisible())
       continue;
 

test/CodeGenCXX/dllexport.cpp

-// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases -disable-llvm-optzns -o - %s -w | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O0 -o - %s -w | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases -disable-llvm-optzns -o - %s -w -fms-compatibility-version=19.00 | FileCheck --check-prefix=MSC --check-prefix=M32 -check-prefix=MSVC2015 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases -disable-llvm-optzns -o - %s -w -fms-compatibility-version=18.00 | FileCheck --check-prefix=MSC --check-prefix=M32 -check-prefix=MSVC2013 %s
+
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O0 -o - %s -w -fms-compatibility-version=19.00 | FileCheck --check-prefix=MSC --check-prefix=M64 -check-prefix=MSVC2015 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O0 -o - %s -w -fms-compatibility-version=18.00 | FileCheck --check-prefix=MSC --check-prefix=M64 -check-prefix=MSVC2013 %s
+
 // RUN: %clang_cc1 -triple i686-windows-gnu    -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O0 -o - %s -w | FileCheck --check-prefix=GNU --check-prefix=G32 %s
 // RUN: %clang_cc1 -triple x86_64-windows-gnu  -emit-llvm -std=c++1y -fno-threadsafe-statics -fms-extensions -O0 -o - %s -w | FileCheck --check-prefix=GNU --check-prefix=G64 %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Exported {};
 struct ExplicitDecl_Exported {};
 struct ExplicitInst_Exported {};
 struct ExplicitSpec_Exported {};

 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_FNestedOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_FNestedInner@NestedOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
 
 template <typename T>
 struct SomeTemplate {
   SomeTemplate(T o = T()) : o(o) {}
   T o;
 };
+// MSVC2015-DAG: define weak_odr dllexport {{.+}} @"\01??4?$SomeTemplate@H@@Q{{.+}}@$$Q{{.+}}@@Z"
+// MSVC2013-DAG: define weak_odr dllexport {{.+}} @"\01??4?$SomeTemplate@H@@Q{{.+}}0@A{{.+}}0@@Z"
 struct __declspec(dllexport) InheritFromTemplate : SomeTemplate<int> {};
 
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_F?$SomeTemplate@H@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
 
 namespace PR23801 {
 template <typename>
 struct S {
   ~S() {}

 // G32-DAG: @_ZTV1W = dllexport unnamed_addr constant [3 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1W to i8*), i8* bitcast (void (%struct.W*)* @_ZN1W3fooEv to i8*)]
 
 struct __declspec(dllexport) X : public virtual W {};
 // vbtable:
 // M32-DAG: @"\01??_8X@@7B@" = weak_odr dllexport unnamed_addr constant [2 x i32] [i32 0, i32 4]
 
 struct __declspec(dllexport) Y {
   // Move assignment operator:
-  // M32-DAG: define weak_odr dllexport x86_thiscallcc dereferenceable({{[0-9]+}}) %struct.Y* @"\01??4Y@@QAEAAU0@$$QAU0@@Z"
+  // MSVC2015-DAG: define weak_odr dllexport {{.+}} @"\01??4Y@@Q{{.+}}@$$Q{{.+}}@@Z"
+  // MSVC2013-DAG: define weak_odr dllexport {{.+}} @"\01??4Y@@Q{{.+}}0@A{{.+}}0@@Z"
 
   int x;
 };
 
 struct __declspec(dllexport) Z { virtual ~Z() {} };
 // The scalar deleting dtor does not get exported:
 // M32-DAG: define linkonce_odr x86_thiscallcc i8* @"\01??_GZ@@UAEPAXI@Z"
 

 
 template <typename T> struct ExplicitInstantiationDeclTemplateBase2 { void func() {} };
 extern template struct ExplicitInstantiationDeclTemplateBase2<int>;
 struct __declspec(dllexport) DerivedFromExplicitInstantiationDeclTemplateBase2 : public ExplicitInstantiationDeclTemplateBase2<int> {};
 template struct __declspec(dllimport) ExplicitInstantiationDeclTemplateBase2<int>;
 USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2<int>, func)
 // M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?func@?$ExplicitInstantiationDeclTemplateBase2@H@@QAEXXZ"
 // G32-DAG: define weak_odr x86_thiscallcc void @_ZN38ExplicitInstantiationDeclTemplateBase2IiE4funcEv
+
+class __declspec(dllexport) ACE_Shared_Object {
+  public:
+    virtual ~ACE_Shared_Object ();
+};
+
+class __declspec(dllexport) ACE_Service_Object: public ACE_Shared_Object {};
+
+// Implicit move constructor declaration.
+// MSVC2015-DAG: define weak_odr dllexport {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
+// The declarations should not be exported.
+// MSVC2013-NOT: define weak_odr dllexport {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q