[PATCH] D109895: [lld-macho][nfc] Construct CFString literals by copying the ConcatInputSection

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:238-239
+static Optional<size_t> getRecordSize(StringRef segname, StringRef name) {
+  if (name == section_names::cfString)
+    if (config->icfLevel != ICFLevel::none && segname == segment_names::data)
+      return target->wordSize == 8 ? 32 : 16;
----------------
oontvoo wrote:
> why not `&&` here rather than nested if?
because there's an incoming `else if` clause in the next diff :)


================
Comment at: lld/MachO/InputFiles.cpp:303
       subsections.push_back({{0, isec}});
-    } else if (config->icfLevel != ICFLevel::none &&
-               (name == section_names::cfString &&
-                segname == segment_names::data)) {
-      uint64_t literalSize = target->wordSize == 8 ? 32 : 16;
-      subsections.push_back({});
-      SubsectionMap &subsecMap = subsections.back();
-      for (uint64_t off = 0; off < data.size(); off += literalSize)
-        subsecMap.push_back(
-            {off, make<ConcatInputSection>(segname, name, this,
-                                           data.slice(off, literalSize), align,
-                                           flags)});
+    } else if (auto recordSize = getRecordSize(segname, name)) {
+      splitRecords(*recordSize);
----------------
oontvoo wrote:
> nit: no braces 
I think convention is to include braces if other clauses of the same `if` chain need braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109895