[PATCH] D36201: Merge manifest namespaces.

[Eric Beckmann via Phabricator via llvm-commits]

ecbeckmann marked an inline comment as done.
ecbeckmann added inline comments.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:106-107
+  for (auto &Arg : InputArgs) {
+    if (!(Arg->getOption().matches(OPT_unsupported) ||
+          Arg->getOption().matches(OPT_supported))) {
+      reportError(Twine("invalid option ") + Arg->getSpelling());
----------------
ruiu wrote:
> `!A && !B` is preferred over `!(A || B)`.
> 
> But do you have to do this way? I think usually we handle OPT_UNKNOWN to report unknown options instead of putting all known options to some groups. Look at https://github.com/llvm-project/llvm-project-20170507/blob/dd24779ef3e3e592cc0c7561a2fb1fbe3309fa1c/lld/ELF/DriverUtils.cpp#L112 for example.
I have tried this....however for some reason OPT_UNKNOWN never filters any arguments for me.  It seems for some reason the invalid flags are recognized as inputs, not flags.  I'm not sure if I'm setting up the options incorrectly or if this is a bug in the option library.....also not sure if this is worth the effort to fix.


https://reviews.llvm.org/D36201