clang-cl: Take dllexport from original function decl into account

[sberg]

Summary

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

[rnk]

Hans knows all about dllexport now.

[hans]

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 <line:7:1, col:26> col:18 used func 'void (void)'
  `-CompoundStmt 0x5ba1de0 <col:25, col:26>

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

`-CXXMethodDecl 0x6af2a08 parent 0x6aa0288 prev 0x6aa04c0 <line:7:1, col:26> col:18 used func 'void (void)'
  |-CompoundStmt 0x6af2b30 <col:25, col:26>
  `-DLLExportAttr 0x6af2b20 <line:1:19> 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 </tmp/a.cc:1:1, line:6:1> line:1:9 struct Befriended definition
| |-CXXRecordDecl 0x6fe43a0 <col:1, col:9> col:9 implicit struct Befriended
| |-CXXMethodDecl 0x6fe4480 <line:2:3, col:42> col:37 func 'void (void)' static
| | `-DLLExportAttr 0x6fe4528 <col:26>
| `-CXXRecordDecl 0x6fe4560 <line:3:3, line:5:3> line:3:10 struct Befriending definition
|   |-CXXRecordDecl 0x6fe4670 <col:3, col:10> col:10 implicit struct Befriending
|   `-FriendDecl 0x6fe4868 <line:4:5, col:34> col:29
|     `-CXXMethodDecl 0x6fe4740 parent 0x6fe4288 prev 0x6fe4480 <col:5, col:34> col:29 func 'void (void)'
|       `-DLLExportAttr 0x6fe4858 <line:2:26> Inherited
`-CXXMethodDecl 0x6fe48c8 parent 0x6fe4288 prev 0x6fe4740 <line:7:1, col:26> col:18 func 'void (void)'
  |-CompoundStmt 0x7036710 <col:25, col:26>
  `-DLLExportAttr 0x6fe49e0 <line:2:26> Inherited

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

[sberg]

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.

[sberg]

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.

[rnk]

lgtm

clang/lib/Sema/SemaDeclCXX.cpp

for (Decl *Member : Class->decls()) {
     if (!cast<NamedDecl>(Member)->isExternallyVisible())
       continue;
 
     if (!getDLLAttr(Member)) {
       auto *NewAttr =
           cast<InheritableAttr>(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<InheritableAttr>(ClassAttr->clone(getASTContext()));
+          NewAttr->setInherited(true);
+          FD->addAttr(NewAttr);
+        }
+      }
     }
   }
 
   if (ClassExported)
     DelayedDllExportClasses.push_back(Class);
 }
 
 /// \brief Perform propagation of DLL attributes from a derived class to a

clang/test/CodeGenCXX/dllexport.cpp

 // GNU-DAG: define dso_local dllexport void @_Z7friend2v()
 struct FuncFriend {
   friend __declspec(dllexport) void friend1();
   friend __declspec(dllexport) void friend2();
 };
 __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]}}(
 void* alloc(__SIZE_TYPE__ n);
 __declspec(dllexport) void* operator new(__SIZE_TYPE__ n) { return alloc(n); }
 
 // MSC-DAG: define dso_local dllexport void @"\01?externalFunc@ns@@YAXXZ"()
 // GNU-DAG: define dso_local dllexport void @_ZN2ns12externalFuncEv()