[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2018-03-07 Thread Stephan Bergmann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326990: Propagate DLLAttr to friend re-declarations of 
member functions (authored by sberg, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D16632?vs=137374=137548#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D16632

Files:
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/test/CodeGenCXX/dllexport.cpp


Index: cfe/trunk/test/CodeGenCXX/dllexport.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp
@@ -290,6 +290,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dso_local dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dso_local dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dso_local dllexport noalias i8* 
@"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dso_local dllexport noalias i8* @_Znw{{[yj]}}(
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -5685,6 +5685,21 @@
   cast(ClassAttr->clone(getASTContext()));
   NewAttr->setInherited(true);
   Member->addAttr(NewAttr);
+
+  if (MD) {
+// Propagate DLLAttr to friend re-declarations of MD that have already
+// been constructed.
+for (FunctionDecl *FD = MD->getMostRecentDecl(); FD;
+ FD = FD->getPreviousDecl()) {
+  if (FD->getFriendObjectKind() == Decl::FOK_None)
+continue;
+  assert(!getDLLAttr(FD) &&
+ "friend re-decl should not already have a DLLAttr");
+  NewAttr = cast(ClassAttr->clone(getASTContext()));
+  NewAttr->setInherited(true);
+  FD->addAttr(NewAttr);
+}
+  }
 }
   }
 


Index: cfe/trunk/test/CodeGenCXX/dllexport.cpp
===
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp
@@ -290,6 +290,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dso_local dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dso_local dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dso_local dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dso_local dllexport noalias i8* @_Znw{{[yj]}}(
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -5685,6 +5685,21 @@
   cast(ClassAttr->clone(getASTContext()));
   NewAttr->setInherited(true);
   Member->addAttr(NewAttr);
+
+  if (MD) {
+// Propagate DLLAttr to friend re-declarations of MD that have already
+// been constructed.
+for (FunctionDecl *FD = MD->getMostRecentDecl(); FD;
+ FD = FD->getPreviousDecl()) {
+  if (FD->getFriendObjectKind() == Decl::FOK_None)
+continue;
+  assert(!getDLLAttr(FD) &&
+ "friend re-decl should not already have a DLLAttr");
+  NewAttr = cast(ClassAttr->clone(getASTContext()));
+  NewAttr->setInherited(true);
+  FD->addAttr(NewAttr);
+}
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2018-03-07 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D16632



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

[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2018-03-07 Thread Stephan Bergmann via Phabricator via cfe-commits 
sberg updated this revision to Diff 137374.
sberg added a comment.

Turns out DLLAttr-inherited-from-class is only added to members during 
Sema::CheckCompletedClass -> Sema::checkClassLevelDLLAttribute, when friend 
re-decls of those members may already have been created.


https://reviews.llvm.org/D16632

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport.cpp


Index: clang/test/CodeGenCXX/dllexport.cpp
===
--- clang/test/CodeGenCXX/dllexport.cpp
+++ clang/test/CodeGenCXX/dllexport.cpp
@@ -290,6 +290,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dso_local dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dso_local dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dso_local dllexport noalias i8* 
@"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dso_local dllexport noalias i8* @_Znw{{[yj]}}(
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5688,6 +5688,21 @@
   cast(ClassAttr->clone(getASTContext()));
   NewAttr->setInherited(true);
   Member->addAttr(NewAttr);
+
+  if (MD) {
+// Propagate DLLAttr to friend re-declarations of MD that have already
+// been constructed.
+for (FunctionDecl *FD = MD->getMostRecentDecl(); FD;
+ FD = FD->getPreviousDecl()) {
+  if (FD->getFriendObjectKind() == Decl::FOK_None)
+continue;
+  assert(!getDLLAttr(FD) &&
+ "friend re-decl should not already have a DLLAttr");
+  NewAttr = cast(ClassAttr->clone(getASTContext()));
+  NewAttr->setInherited(true);
+  FD->addAttr(NewAttr);
+}
+  }
 }
   }
 


Index: clang/test/CodeGenCXX/dllexport.cpp
===
--- clang/test/CodeGenCXX/dllexport.cpp
+++ clang/test/CodeGenCXX/dllexport.cpp
@@ -290,6 +290,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dso_local dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dso_local dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dso_local dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dso_local dllexport noalias i8* @_Znw{{[yj]}}(
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5688,6 +5688,21 @@
   cast(ClassAttr->clone(getASTContext()));
   NewAttr->setInherited(true);
   Member->addAttr(NewAttr);
+
+  if (MD) {
+// Propagate DLLAttr to friend re-declarations of MD that have already
+// been constructed.
+for (FunctionDecl *FD = MD->getMostRecentDecl(); FD;
+ FD = FD->getPreviousDecl()) {
+  if (FD->getFriendObjectKind() == Decl::FOK_None)
+continue;
+  assert(!getDLLAttr(FD) &&
+ "friend re-decl should not already have a DLLAttr");
+  NewAttr = cast(ClassAttr->clone(getASTContext()));
+  NewAttr->setInherited(true);
+  FD->addAttr(NewAttr);
+}
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2016-01-27 Thread Stephan Bergmann via cfe-commits 
sberg created this revision.
sberg added a reviewer: rnk.
sberg added a subscriber: cfe-commits.

...in cases where a member function is redeclared as a friend of a nested 
class.  (LibreOffice happens to get tripped up by this.)

http://reviews.llvm.org/D16632

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGenCXX/dllexport.cpp

Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -264,6 +264,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}(
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -765,7 +765,8 @@
 
   if (FD->hasAttr())
 F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
-  else if (FD->hasAttr())
+  else if (FD->hasAttr() ||
+   FD->getCanonicalDecl()->hasAttr())
 F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
   else
 F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);


Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -264,6 +264,16 @@
 __declspec(dllexport) void friend1() {}
   void friend2() {}
 
+// MSC-DAG: define dllexport void @"\01?func@Befriended@@SAXXZ"()
+// GNU-DAG: define dllexport void @_ZN10Befriended4funcEv()
+struct __declspec(dllexport) Befriended {
+  static void func();
+  struct Befriending {
+friend void Befriended::func();
+  };
+};
+void Befriended::func() {}
+
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}(
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -765,7 +765,8 @@
 
   if (FD->hasAttr())
 F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
-  else if (FD->hasAttr())
+  else if (FD->hasAttr() ||
+   FD->getCanonicalDecl()->hasAttr())
 F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
   else
 F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2016-01-27 Thread Reid Kleckner via cfe-commits 
rnk added a subscriber: rnk.
rnk added a comment.

Hans knows all about dllexport now.


http://reviews.llvm.org/D16632



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

Re: [PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2016-01-27 Thread Stephan Bergmann via cfe-commits 
sberg added a comment.

Yeah, my first naive finding when encountering the error was that it went away 
when unconditionally using FD->getCanonicalDecl() instead of FD in that 
if-else-if block. But that caused other parts of clang-test to fail. The 
current version passes all tests (happens to), but indeed does not feel "right" 
at all.


http://reviews.llvm.org/D16632



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

Re: [PATCH] D16632: clang-cl: Take dllexport from original function decl into account

2016-01-27 Thread Hans Wennborg via cfe-commits 
hans added a comment.

Hi Stephan,

I would rather see that we could get this right in the AST.

The problem is that the Befriended::func() definition doesn't have dllexport 
attached:

  `-CXXMethodDecl 0x5ba1cf0 parent 0x5b4f288 prev 0x5b4f750  
col:18 used func 'void (void)'
`-CompoundStmt 0x5ba1de0 

If I drop the friend declaration, it looks like this:

  `-CXXMethodDecl 0x6af2a08 parent 0x6aa0288 prev 0x6aa04c0  
col:18 used func 'void (void)'
|-CompoundStmt 0x6af2b30 
`-DLLExportAttr 0x6af2b20  Inherited

It's a tricky situation though. I suspect what's happening is that when we 
build the AST for the friend decl, we haven't yet propagated the dllexport 
attribute from the 'Befriended' class to 'func'.

If the attribute was put directly on 'func', the friend declaration would pick 
it up:

  struct  Befriended {
static void __declspec(dllexport) func();
struct Befriending {
  friend void Befriended::func();
};
  };
  void Befriended::func() {}
  
  |-CXXRecordDecl 0x6fe4288  line:1:9 struct 
Befriended definition
  | |-CXXRecordDecl 0x6fe43a0  col:9 implicit struct Befriended
  | |-CXXMethodDecl 0x6fe4480  col:37 func 'void (void)' 
static
  | | `-DLLExportAttr 0x6fe4528 
  | `-CXXRecordDecl 0x6fe4560  line:3:10 struct Befriending 
definition
  |   |-CXXRecordDecl 0x6fe4670  col:10 implicit struct 
Befriending
  |   `-FriendDecl 0x6fe4868  col:29
  | `-CXXMethodDecl 0x6fe4740 parent 0x6fe4288 prev 0x6fe4480  col:29 func 'void (void)'
  |   `-DLLExportAttr 0x6fe4858  Inherited
  `-CXXMethodDecl 0x6fe48c8 parent 0x6fe4288 prev 0x6fe4740  
col:18 func 'void (void)'
|-CompoundStmt 0x7036710 
`-DLLExportAttr 0x6fe49e0  Inherited

We need to figure out the ordering of applying the dllexport class attribute 
vs. parsing inner classes.


http://reviews.llvm.org/D16632



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