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

Nico Rieck nico.rieck at gmail.com
Tue May 27 21:16:32 PDT 2014


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.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4375
@@ +4374,3 @@
+    // FIXME: Warn: the returned class should probably be exported.
+  }
+
----------------
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.

================
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
----------------
I wonder what MS exactly means by that since it's valid to inherit from a non-exported class or even an imported class.

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

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

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

http://reviews.llvm.org/D3877






More information about the cfe-commits mailing list