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

[George Rimar via Phabricator via llvm-commits]

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

This is LGTM as is, but also added a suggestion about possible simplification.
I have no strong preference here though. I assume no much people will use this option anyways.



================
Comment at: lld/ELF/Driver.cpp:944
+  // For --{push,pop}-state
+  std::vector<std::tuple<bool, bool, bool>> Stack;
+
----------------
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.


================
Comment at: lld/ELF/Driver.cpp:1032
+      std::tie(Config->AsNeeded, Config->Static, InWholeArchive) = Stack.back();
+      Stack.pop_back();
+      break;
----------------
Then it can be probably a bit simpler:

```
if (!Stack))
  error("--pop-state used without --push-state");
std::tie(Config->AsNeeded, Config->Static, InWholeArchive) =
  Stack.getValueOr(std::tuple<bool, bool, bool>());
break;
```


https://reviews.llvm.org/D47542