[PATCH] Start adding support for dllimport/dllexport on classes (PR11170)

[Hans Wennborg]

Thanks for the review!

> I also have a few more tests you should use. For example currently sema diagnoses dllimport redeclarations and drops this attribute, but class template member functions would still be imported.

I'd like to look into the whole redeclaration business in a separate patch. It seems the rules are different for redeclaring classes and other things are different with respect to these attributes.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4375
@@ +4374,3 @@
+    // FIXME: Warn: the returned class should probably be exported.
+  }
+
----------------
Nico Rieck wrote:
> You picked this from my old patch but I now think that this diagnostic wouldn't buy much. MSVC doesn't do it, and it's perfectly valid to return for example value objects. With this removed I just inlined this whole function into the loop below.
Sounds good to me.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4393
@@ +4392,3 @@
+
+  // FIXME: All bases must be exportable. MSVC doesn't seem to warn about
+  // this, but we should. We also need to propagate the attribute upwards to
----------------
Nico Rieck wrote:
> I wonder what MS exactly means by that since it's valid to inherit from a non-exported class or even an imported class.
It's pretty confusing.. Their docs on http://msdn.microsoft.com/en-us/library/81h27t8c.aspx specifically say "All base classes of an exportable class must be exportable. If not, a compiler warning is generated."

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4412
@@ +4411,3 @@
+          // Instantiate non-default methods.
+          S.MarkFunctionReferenced(Class->getLocation(), MD);
+        } else if (!MD->isDeleted() &&
----------------
Nico Rieck wrote:
> When is this branch actually needed?
It's needed for this case:

template <typename T> struct __declspec(dllexport) U { void foo() {} };
struct __declspec(dllexport) V : public U<int> { };

to make us instantiate U<int>::foo().

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4413
@@ +4412,3 @@
+          S.MarkFunctionReferenced(Class->getLocation(), MD);
+        } else if (!MD->isDeleted() &&
+                   (!MD->isTrivial() || MD->isCopyAssignmentOperator())) {
----------------
Nico Rieck wrote:
> This check is redundant.
Thanks! Removed.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4424
@@ +4423,3 @@
+      }
+    }
+  }
----------------
Nico Rieck wrote:
> This fails to export defaulted members for the MS abi (Mingw currently does not export them).
Thanks for catching that!

http://reviews.llvm.org/D3877