[PATCH] D114017: [lld-macho][nfc] Factor-out NFC changes from main __eh_frame diff

Vincent Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 17 11:25:43 PST 2021


thevinster added a comment.

Minor nits and stuff. Overall, generally looks good



================
Comment at: lld/MachO/Driver.cpp:1028
+        continue;
+      if (subsections[0].isec->getName() == section_names::compactUnwind)
+        // Compact unwind entries require special handling elsewhere.
----------------
Do we also need to check for `segment_names::ld` here as well? or can that be assumed that compact_unwind is always under __ld? 


================
Comment at: lld/MachO/InputFiles.cpp:278
             " is too large");
-      sections.push_back({});
+      sections.push_back(sec.addr);
       continue;
----------------
Don't you need braces on these surrounding the `sec.addr`? I'm surprised that the push_back supports constructor initialization via its arguments (emplace_back supports it though).


================
Comment at: lld/MachO/InputSection.h:306
 constexpr const char ehFrame[] = "__eh_frame";
+constexpr const char gccExceptTab[] = "__gcc_except_tab";
 constexpr const char export_[] = "__export";
----------------
nit: Generally, it would be nice to see variables also get defined and used in the same diff to make things more "self-contained". I imagine this will be used in your next diff? No strong opinions though. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114017



More information about the llvm-commits mailing list