[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:136
+  // Otherwise just check the clang file name.
+  return llvm::sys::path::stem(CmdLine.front()).endswith_lower("clang-cl");
+}
----------------
We support being called "CL.exe", but with our new VS integration, I don't think it matters to check for this case anymore. We may remove it over time.


================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:140
-    unsigned MissingI, MissingC;
-    auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
-    for (const auto *Arg : ArgList) {
----------------
Can you not keep using ParseArgs? It takes FlagsToInclude and FlagsToExclude, which you can compute outside the loop now.


================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:156-169
+    // Transform the command line to an llvm ArgList.
+    // Make sure that OldArgs lives for at least as long as this variable as the
+    // arg list contains pointers to the OldArgs strings.
+    llvm::opt::InputArgList ArgList;
+    {
+      // Unfortunately InputArgList requires an array of c-strings whereas we
+      // have an array of std::string; we'll need an intermediate vector.
----------------
I think the old for loop was nicer. I don't think this code needs to change, you should be able to use ParseArgs with the extra flag args.


https://reviews.llvm.org/D51321