[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.


================
Comment at: clang/include/clang/Frontend/CompilerInvocation.h:156
   /// \param [out] Res - The resulting invocation.
+  /// \param [in] CommandLineArgs - Array of argument strings, this should not
+  /// contain "-cc1".
----------------
Is this really a should not, or is it must not?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3604-3610
+#define OPTION_WITH_MARSHALLING(PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS,     \
+                                ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR,    \
+                                VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   \
+  if (Option::KIND##Class == Option::FlagClass)                                \
+    Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
+#include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING
----------------
This should probably go in a separate function as it will grow. I would recommend something like `ParseSimpleArgs` and call it right before `ParseAnalyzerArgs `.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3607
+                                VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE)   \
+  if (Option::KIND##Class == Option::FlagClass)                                \
+    Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
----------------
How would this handle other option classes? I think it would be good to include a few different types of options in the first patch.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3845
+      IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)          \
+    Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+#include "clang/Driver/Options.inc"
----------------
It's a little sad that we need to allocation every string just because of the `-`. We definitely need to be able to allocate strings for options with data, but it would be good if we could just have the strings with `-` prefixed in the option table if that's reasonable to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79796