aaron.ballman added a comment. In https://reviews.llvm.org/D23455#517774, @alexfh wrote:
> In https://reviews.llvm.org/D23455#516760, @zturner wrote: > > > In https://reviews.llvm.org/D23455#516753, @alexfh wrote: > > > > > In https://reviews.llvm.org/D23455#515527, @rnk wrote: > > > > > > > In https://reviews.llvm.org/D23455#515486, @brad.king wrote: > > > > > > > > > > the feasibility of emitting 'arguments' instead of 'command' into > > > > > > the JSON compilation database. > > > > > > > > > > > > > > > CMake constructs the command lines internally using string > > > > > replacement on templates. We never actually know the exact > > > > > arguments. Therefore providing arguments instead of the whole > > > > > command would require parsing to be done on the CMake side instead. > > > > > This is theoretically possible because we do know the shell for which > > > > > we are generating (Windows `cmd` versus MSYS `sh`). However, it may > > > > > also require a bunch of logic we don't have yet but that LLVM does. > > > > > > > > > > Alternatively, the JSON could have an additional > > > > > `command_shell="..."` field that indicates the shell for which the > > > > > command line is encoded. > > > > > > > > > > > > Bummer. Given that this is hard to do in CMake, then I think we should > > > > just tokenize in Clang. Let's use llvm::sys::getProcessTriple() instead > > > > of LLVM_ON_WIN32 and check if that is an MSVC environment as a proxy > > > > for the shell type. > > > > > > > > > Do I understand correctly that `llvm::sys::getProcessTriple()` returns > > > basically a compile-time constant? If yes, it won't allow the same clang > > > tool binary to be used with both command line formats. Should we instead > > > allow runtime selection of the command line format? For example, by > > > extending JSON compilation database with the aforementioned > > > `command_shell="..."` option. > > > > > > It does return a compile time constant, but it is a compile time constant > > representing the platform that clang-tidy is running on. I don't see a > > need to have a single clang-tidy binary be able to parse both command line > > formats. In fact, it seems actively harmful. > > > > If you are running on a Windows system with windows command line parsing > > rules, and someone has generated a command line on linux, then this command > > line was never intended to be run on Windows in the first place. The JSON > > compilation database spec already says that the command line represents > > "the exact command to be run **in the environment of the build system**". > > By adding the flexibility to change the environment, this is no longer > > true. If I try to run a command generated for one build system in the > > environment of another build system, even if I tokenize it correctly whose > > to say it will work? > > > > I understand the desire to make sure we get the right change so we don't > > have to revisit this in the future, but to me this sounds like YAGNI and > > actually increasing the risk of someone using the software and getting > > unexpected results, not less risk. > > > Now I'm less sure. And I might be misunderstanding something here. Folks > actually using clang tools on Windows (Aaron?) can tell with more confidence > whether Linux-style command line tokenization is of any use on Windows. I had > an impression that it allowed clang tools to be used with mingw (and msys > shell), but I'm not sure whether it's an important use case for anyone. I agree with @zturner's perspective -- if the command was generated on Linux, I would not expect it to work on Windows (and vice versa). Not only are path separators an issue, you run into other things like reserved file names, different -D flags, different triples, etc. I can't think of a situation where I would expect that to work. https://reviews.llvm.org/D23455 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
