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

[Dustin L. Howett via Phabricator via cfe-commits]

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 indirection on class pointers, there may be issues left to iron out.



================
Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188
 
+  StringRef SymbolPrefix() {
+    return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";
----------------
Should this be `SymbolPrefix()` or something more like `MangleSymbol(StringRef sym)`? I know that right now we only need to prepend `_` or `._`, but is it more future-proof to make it generic?


================
Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1528
     InitStructBuilder.addInt(Int64Ty, 0);
-    for (auto *s : SectionsBaseNames) {
-      auto bounds = GetSectionBounds(s);
-      InitStructBuilder.add(bounds.first);
-      InitStructBuilder.add(bounds.second);
-    };
+    if (CGM.getTriple().isOSBinFormatCOFF()) {
+      for (auto *s : PECOFFSectionsBaseNames) {
----------------
We may be able to reduce the duplication here (aside: it is very cool that Phabricator shows duplicated lines) by capturing `auto sectionVec = &SectionsBaseNames;` and switching which is in use based on bin format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724