[PATCH] D113235: [lld-macho] Replace LC_LINKER_OPTION parsing

[Keith Smiley via Phabricator via llvm-commits]

keith marked 2 inline comments as done.
keith added inline comments.


================
Comment at: lld/MachO/Driver.cpp:419
                  /*isReexport=*/false, /*isExplicit=*/false, forceLoadArchive);
       break;
     }
----------------
int3 wrote:
> keith wrote:
> > int3 wrote:
> > > wait a minute... shouldn't this be `continue`, or the next line changed to `else if`?
> > > 
> > > I'd thought we would have tests to catch this :/
> > Since this parses a single option at a time shouldn't breaking be ok? I guess I'm not sure if the format intends to support `-lfoo -lbar` in a single LC_LINKER_OPTION or not?
> Oh I see... we have one LC_LINKER_OPTION per `-lfoo` and `-framework Foo`.
> 
> Doesn't that mean the `for` loop is redundant, since we'll only have 1 or 2 elements in `argv`?
So I wasn't sure about that given the way argv is setup and explicitly defined as a size of 4. Pushed an option without that which still passes the current tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113235