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

[David Majnemer]

On Tue, May 27, 2014 at 4:51 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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?
>

Given a struct 'A', they export the following as well:
public: struct A & __thiscall A::operator=(struct A &&)


>
> ================
> 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140527/a3156a50/attachment.html>