[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC357364: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs. (authored by theraven, committed by ). Changed

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven updated this revision to Diff 193002. theraven marked 2 inline comments as done. theraven added a comment. - Fix ownership with Twine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58724/new/ https://reviews.llvm.org/D58724 Files:

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + Twine ManglePublicSymbol(StringRef Name) { +return StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name; DHowett-MSFT wrote: > As of the latest revision,

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + Twine ManglePublicSymbol(StringRef Name) { +return StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name; As of the latest revision, this now fails at

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven updated this revision to Diff 15. theraven marked an inline comment as done. theraven added a comment. - Address Dustin's review comments. - [objc-gnustep] Use $ in symbol names on Windows. - Add fix from Dustin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1231 +break; + +auto Storage = llvm::GlobalValue::DefaultStorageClass; After we get the `ObjCInterfaceDecl`, we have to make sure it's the one corresponding to

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; theraven wrote: > DHowett-MSFT wrote: > > theraven wrote: > > > DHowett-MSFT wrote:

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven marked 2 inline comments as done. theraven added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; DHowett-MSFT wrote: > theraven wrote: > >

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; theraven wrote: > DHowett-MSFT wrote: > > Should this be `SymbolPrefix()` or

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven marked 2 inline comments as done. theraven added inline comments. Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188 + StringRef SymbolPrefix() { +return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._"; DHowett-MSFT wrote: > Should this be

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

DHowett-MSFT added a comment. This looks great, and takes up the parts of my patch that I cared about. Thank you for doing this. My primary concern is that the patch includes my "early init" changes -- while I support it and think it's the right solution, especially where it reduces double

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven added a comment. This review is mostly so that @DHowett-MSFT can check that I didn't break his patches too badly in the cleanup and test that they still do allow building Objective-C DLLs on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

theraven created this revision. theraven added a reviewer: DHowett-MSFT. Herald added a project: clang. Herald added a subscriber: cfe-commits. theraven added a comment. This review is mostly so that @DHowett-MSFT can check that I didn't break his patches too badly in the cleanup and test that