[PATCH] D11850: Delay emitting members of dllexport classes until the class is fully parsed (PR23542)

2018-12-18 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

In D11850#1334344 , @aganea wrote:

> Cross-referencing PR40006 . It 
> seems to be a different manifestation of this same bug.
>
> @hans Could you possibly take a look at the bug above whenever you have time? 
> Many thanks!


Sorry, I saw the bug but didn't have time to look at it yet. I will try to get 
to it tomorrow.




Comment at: lib/Sema/SemaDeclCXX.cpp:4700-4703
+  if (TSK == TSK_ExplicitInstantiationDeclaration)
+// Don't go any further if this is just an explicit instantiation
+// declaration.
+continue;

rsmith wrote:
> Can we bail out of the function early in this case?
Yes. Done.



Comment at: lib/Sema/SemaDeclCXX.cpp:9514
+SmallVector WorkList =
+std::move(DelayedDllExportClasses);
+for (CXXRecordDecl *Class : WorkList)

I was trying to fancy and use C++11 here, but I think I really just want 
good-old std::swap, because IIUC, I'm not supposed to use a var after 
std::move'ing from it?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D11850/new/

https://reviews.llvm.org/D11850



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D11850: Delay emitting members of dllexport classes until the class is fully parsed (PR23542)

2018-12-18 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added subscribers: belkiss, rnk, aganea.
aganea added a comment.
Herald added a subscriber: llvm-commits.

Cross-referencing PR40006 . It 
seems to be a different manifestation of this same bug.

@hans Could you possibly take a look at the bug above whenever you have time? 
Many thanks!


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D11850/new/

https://reviews.llvm.org/D11850



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11850: Delay emitting members of dllexport classes until the class is fully parsed (PR23542)

2015-08-14 Thread Richard Smith via cfe-commits 
rsmith added inline comments.


Comment at: lib/Sema/SemaDeclCXX.cpp:4700-4703
@@ +4699,6 @@
+if (Member-getAttrDLLExportAttr()) {
+  if (TSK == TSK_ExplicitInstantiationDeclaration)
+// Don't go any further if this is just an explicit instantiation
+// declaration.
+continue;
+

Can we bail out of the function early in this case?


Comment at: lib/Sema/SemaDeclCXX.cpp:9501
@@ -9481,3 +9500,3 @@
 
 void Sema::ActOnFinishCXXMemberDefaultArgs(Decl *D) {
   auto *RD = dyn_castCXXRecordDecl(D);

Can you rename this to something more general, like 
`ActOnFinishCXXNonNestedClass`, now that we're using it to check things other 
than default arguments?


http://reviews.llvm.org/D11850



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11850: Delay emitting members of dllexport classes until the class is fully parsed (PR23542)

2015-08-14 Thread Hans Wennborg via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL245139: Delay emitting members of dllexport classes until 
the class is fully parsed… (authored by hans).

Changed prior to commit:
  http://reviews.llvm.org/D11850?vs=32201id=32207#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D11850

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
  cfe/trunk/test/CodeGenCXX/dllexport.cpp

Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -4682,6 +4682,60 @@
   }
 }
 
+static void ReferenceDllExportedMethods(Sema S, CXXRecordDecl *Class) {
+  Attr *ClassAttr = getDLLAttr(Class);
+  if (!ClassAttr)
+return;
+
+  assert(ClassAttr-getKind() == attr::DLLExport);
+
+  TemplateSpecializationKind TSK = Class-getTemplateSpecializationKind();
+
+  if (TSK == TSK_ExplicitInstantiationDeclaration)
+// Don't go any further if this is just an explicit instantiation
+// declaration.
+return;
+
+  for (Decl *Member : Class-decls()) {
+auto *MD = dyn_castCXXMethodDecl(Member);
+if (!MD)
+  continue;
+
+if (Member-getAttrDLLExportAttr()) {
+  if (MD-isUserProvided()) {
+// Instantiate non-default class member functions ...
+
+// .. except for certain kinds of template specializations.
+if (TSK == TSK_ImplicitInstantiation  !ClassAttr-isInherited())
+  continue;
+
+S.MarkFunctionReferenced(Class-getLocation(), MD);
+
+// The function will be passed to the consumer when its definition is
+// encountered.
+  } else if (!MD-isTrivial() || MD-isExplicitlyDefaulted() ||
+ MD-isCopyAssignmentOperator() ||
+ MD-isMoveAssignmentOperator()) {
+// Synthesize and instantiate non-trivial implicit methods, explicitly
+// defaulted methods, and the copy and move assignment operators. The
+// latter are exported even if they are trivial, because the address of
+// an operator can be taken and should compare equal accross libraries.
+DiagnosticErrorTrap Trap(S.Diags);
+S.MarkFunctionReferenced(Class-getLocation(), MD);
+if (Trap.hasErrorOccurred()) {
+  S.Diag(ClassAttr-getLocation(), diag::note_due_to_dllexported_class)
+   Class-getName()  !S.getLangOpts().CPlusPlus11;
+  break;
+}
+
+// There is no later point when we will see the definition of this
+// function, so pass it to the consumer now.
+S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
+  }
+}
+  }
+}
+
 /// \brief Check class-level dllimport/dllexport attribute.
 void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
   Attr *ClassAttr = getDLLAttr(Class);
@@ -4783,45 +4837,10 @@
   NewAttr-setInherited(true);
   Member-addAttr(NewAttr);
 }
-
-if (MD  ClassExported) {
-  if (TSK == TSK_ExplicitInstantiationDeclaration)
-// Don't go any further if this is just an explicit instantiation
-// declaration.
-continue;
-
-  if (MD-isUserProvided()) {
-// Instantiate non-default class member functions ...
-
-// .. except for certain kinds of template specializations.
-if (TSK == TSK_ImplicitInstantiation  !ClassAttr-isInherited())
-  continue;
-
-MarkFunctionReferenced(Class-getLocation(), MD);
-
-// The function will be passed to the consumer when its definition is
-// encountered.
-  } else if (!MD-isTrivial() || MD-isExplicitlyDefaulted() ||
- MD-isCopyAssignmentOperator() ||
- MD-isMoveAssignmentOperator()) {
-// Synthesize and instantiate non-trivial implicit methods, explicitly
-// defaulted methods, and the copy and move assignment operators. The
-// latter are exported even if they are trivial, because the address of
-// an operator can be taken and should compare equal accross libraries.
-DiagnosticErrorTrap Trap(Diags);
-MarkFunctionReferenced(Class-getLocation(), MD);
-if (Trap.hasErrorOccurred()) {
-  Diag(ClassAttr-getLocation(), diag::note_due_to_dllexported_class)
-   Class-getName()  !getLangOpts().CPlusPlus11;
-  break;
-}
-
-// There is no later point when we will see the definition of this
-// function, so pass it to the consumer now.
-Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
-  }
-}
   }
+
+  if (ClassExported)
+DelayedDllExportClasses.push_back(Class);
 }
 
 /// \brief Perform propagation of DLL attributes from a derived class to a
@@ -9479,14 +9498,23 @@
   }
 }
 
-void Sema::ActOnFinishCXXMemberDefaultArgs(Decl *D) {