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

Dustin L. Howett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 27 13:51:04 PST 2019

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list