[PATCH] D109944: [lld-macho] Associate compact unwind entries with function symbols

[Greg McGary via Phabricator via llvm-commits]

gkm accepted this revision.
gkm added a comment.
This revision is now accepted and ready to land.

LGTM! Thank you for this. It makes `__eh_frame` handling much nicer too.
Please fix test failure for `MachO/compact-unwind-both-local-and-dylib-personality.s`.



================
Comment at: lld/MachO/InputSection.cpp:104-107
+    } else if ((*it)->value > (*copyIt)->value)
+      std::swap(*it++, *copyIt);
+    else
+      ++it;
----------------
Nit: According to LLVM Coding Standards, if one body of an if/else chain has braces, then all should have them.


================
Comment at: lld/MachO/Writer.cpp:690-695
+  for (const InputFile *file : inputFiles)
+    if (auto *objFile = dyn_cast<ObjFile>(file))
+      for (Symbol *sym : objFile->symbols)
+        if (auto *defined = dyn_cast_or_null<Defined>(sym))
+          if (!defined->isExternal() && defined->isLive())
+            in.unwindInfo->addSymbol(defined);
----------------
Nit: LLVM Coding Standards wants braces when nested statements are more than 2 deep, though there is no clear guidance about where to put them here. My suggestion is around the `for` bodies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109944