[PATCH] D76839: [lld-macho] Extend SyntheticSections to cover all segment load commands

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:259-261
+    return linkEditOffset + 1;
+  else
+    return linkEditOffset;
----------------
int3 wrote:
> smeenai wrote:
> > ruiu wrote:
> > > int3 wrote:
> > > > ruiu wrote:
> > > > > So it looks like the function returns only 4 possible values -- 0, 1, linkEditOffset, or linkEditOffset+1. Can't you use 0, 1, 2, 3 instead?
> > > > The idea is to make it easily extensible -- there may be other special sections that we want to order before the __LINKEDIT sections.
> > > Got it, but I don't think you have to prepare this tiny function for future extension. You can just make it return 0,1,2,3 and then in a later change when you actually need to use linkEditOffset, you can start returning linkEditOffset and linkEditOffset+1.
> > Looks like this comment still needs a response.
> > You can just make it return 0,1,2,3 and then in a later change when you actually need to use linkEditOffset, you can start returning linkEditOffset and linkEditOffset+1.
> 
> I'd actually forgotten to add the symbol table / string pool sections to the ordering after rebasing, oops. So we do actually have a bunch of __LINKEDIT sections to order in this diff already.
> 
> In any case, I've refactored things so that the ordering can be specified a bit more declaratively, though it does add a bit more code... let me know what you think, I can revert to the if-else chain if you'd prefer.
> 
> 
> I'd actually forgotten to add the symbol table / string pool sections to the ordering after rebasing

Nevermind, I've instead swapped the stack position of this diff with that of D76742, so those sections are added later...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76839