zturner added a comment.

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.


https://reviews.llvm.org/D23455



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to