[PATCH] Start adding support for dllimport/dllexport on classes (PR11170)
Richard Smith
richard at metafoo.co.uk
Tue May 27 16:51:26 PDT 2014
Looks basically fine. Just a few comments.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:1395
@@ +1394,3 @@
+ // Don't dllexport/import destructor thunks.
+ F->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
+ }
----------------
This would make more sense to me in `SetFunctionAttributes`, where (presumably) the DLL storage class is set.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:4394
@@ +4393,3 @@
+ // template specialization bases.
+ for (Decl *D : Class->decls()) {
+ if (VarDecl *VD = dyn_cast<VarDecl>(D)) {
----------------
What about member classes? Are they implicitly re-exported too?
================
Comment at: lib/Sema/SemaDeclCXX.cpp:4404
@@ +4403,3 @@
+
+ if (ClassExported && !MD->isDefaulted() && !MD->isDeleted()) {
+ // Instantiate non-default methods.
----------------
`ClassExported && MD->isUserProvided()` maybe
================
Comment at: lib/Sema/SemaDeclCXX.cpp:4407-4408
@@ +4406,4 @@
+ S.MarkFunctionReferenced(Class->getLocation(), MD);
+ } else if (ClassExported && !MD->isDeleted() &&
+ (!MD->isTrivial() || MD->isCopyAssignmentOperator())) {
+ // Also instantiate non-trivial default methods and the copy assignment
----------------
Weird! You get an exported trivial copy assignment but not an exported trivial copy constructor? I guess we'll need to update this once we find out what they do with implicit move operations?
================
Comment at: lib/Sema/SemaDeclCXX.cpp:4415
@@ +4414,3 @@
+ S.ResolveExceptionSpec(Class->getLocation(), FPT);
+ S.ActOnFinishInlineMethodDef(MD);
+ }
----------------
Why do we need this here but not in the non-defaulted case?
http://reviews.llvm.org/D3877
More information about the cfe-commits
mailing list