[PATCH] D132947: [lld-macho] Support synthesizing __TEXT,__init_offsets

[Daniel Bertalan via Phabricator via llvm-commits]

BertalanD marked an inline comment as done.
BertalanD added inline comments.


================
Comment at: lld/MachO/Writer.cpp:1136
+  if (in.initOffsets->isNeeded())
+    in.initOffsets->setup();
 
----------------
thakis wrote:
> Very nit: I'd appreciate it if you could rename the existing two `setup()` methods to `setUp()` in a preparatory commit (just land, no need for review), and then call this new one `setUp` too. ("setup" is a noun, "to set up" is a verb, and methods are named after verbs.)
sgtm


================
Comment at: llvm/lib/MC/MCSectionMachO.cpp:65
      StringLiteral("S_THREAD_LOCAL_INIT_FUNCTION_POINTERS")}, // 0x15
+    {StringLiteral("") /*FIXME??*/,
+     StringLiteral("S_INIT_FUNC_OFFSETS")}, // 0x16
----------------
thakis wrote:
> I wanted to say something about this, but apparently it's local style in this file O_o
I'm considering adding a separate patch that cleans these up. Apparently it was uglified because some old GCC version didn't like implicitly converting a string literal to `StringLiteral`. (https://github.com/llvm/llvm-project/commit/a9df9414030f6cbec511a8cef941998a268b9c6e)

I assume we're way past the "apparently GCC still doesn't understand C++11" situation (at least since the required was bumped to C++17).


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

https://reviews.llvm.org/D132947