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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/Driver.cpp:944
+  // For --{push,pop}-state
+  std::vector<std::tuple<bool, bool, bool>> Stack;
+
----------------
ruiu wrote:
> 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
> 
> 
It just what that comment in their bugzilla seems was saying. But I did not check that.


https://reviews.llvm.org/D47542