hamzasood marked an inline comment as done.
hamzasood 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");
+}
----------------
rnk wrote:
> 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.
Should I add the check anyway?
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:140
- unsigned MissingI, MissingC;
- auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
- for (const auto *Arg : ArgList) {
----------------
rnk wrote:
> Can you not keep using ParseArgs? It takes FlagsToInclude and FlagsToExclude,
> which you can compute outside the loop now.
Good point, I forgot to move the flags out of the loop when moving the
—driver-mode check.
But I don’t think ParseArgs is suitable (see the other comment response).
================
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.
----------------
rnk wrote:
> 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.
The problem with the old loop is that we lose aliasing information (e.g. `/W3`
becomes `-Wall`). While `ParseOneArg` also performs alias conversion, it gives
us indices for each individual argument. The last line of the new loop copies
arguments by using that index information to read from `OldArgs`, which gives
us the original spelling.
https://reviews.llvm.org/D51321
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
