[PATCH] D47542: Implement --{push,pop}-state.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Driver.cpp:944
+  // For --{push,pop}-state
+  std::vector<std::tuple<bool, bool, bool>> Stack;
+
----------------
grimar wrote:
> One idea based on the difference of bfd and gold implementations.
> As far I know, gold implements the true stack, but bfd - don't (https://sourceware.org/bugzilla/show_bug.cgi?id=18989#c2).
> And letting users use the full power of nesting in invocations looks like a road to hell..
> 
> So should we just do it `Optional<std::tuple<bool, bool, bool>> Stack`?
> Also, I would rename to `State` then. 
> (Or `States` if you decide to go with the true stack). It is consistent with the option name.
Looks like bfd linker at least can count the nesting depth. Are you sure that bfd cannot keep the stack of states?

  $ ld.bfd --push-state --push-state --pop-state --pop-state
  ld.bfd: no input files

  $ ld.bfd --push-state --push-state --pop-state --pop-state --pop-state
  ld.bfd: no state pushed before popping




https://reviews.llvm.org/D47542