[PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

[Zachary Turner via cfe-commits]

Also, compilation database support with CMake works correctly on Windows.
It generates Windows command lines which need to be tokenized using Windows
command line rules (hence why this patch makes clang-tidy work on Windows)
On Sat, Aug 13, 2016 at 10:30 PM Zachary Turner <zturner at google.com> wrote:

> I'm not disagreeing that it would be nice if CMake supported this. It's
> probably worth trying to do even.
>
> But do we want to block having a working clang-tidy for clang-cl for
> months because of that? Even if we can upstream the patch, how long before
> we up the minimum required CMake version?
>
> The solution here does not regress behavior on any supported
> configuration, and adds support for a new platform. Copying a compilation
> database from one machine to another seems like a hypothetical edge case.
>
> To support testing perhaps we can make our compilation database parser
> support an optional field in the json that CMake doesn't know about like
> PathSyntax: 'windows'. Again, this seems like something we could do
> iteratively and with low priority because it's not needed in order to
> enable or fix any presently supported use cases.
> On Sat, Aug 13, 2016 at 9:58 PM Manuel Klimek <klimek at google.com> wrote:
>
>>
>>
>> On Sat, Aug 13, 2016, 10:17 PM Zachary Turner <zturner at google.com> wrote:
>>
>>> The json is generated by CMake, so I don't thinks we can do this without
>>> patching CMake, which is a non-starter.
>>>
>>
>> Why? We did add compilation database support to cmake in the first place.
>> Back then I did not support windows, btw, so I'd be surprised if that
>> worked now without also using the arguments format that has already been
>> added to the spec for that reason.
>>
>> I don't think this will break mingw. Mingw is still Windows, and still
>>> uses Windows backslashes, quoting rules, and escaping rules.
>>>
>>> You might be thinking of cygwin, but in the case LLVM_ON_WIN32is not
>>> defined.
>>>
>>> Reid, do you agree with this?
>>>
>>
>> I'd like to see a stronger reason why adding arguments support to cmake
>> is not the right thing to do.
>>
>>
>>> On Sat, Aug 13, 2016 at 10:35 AM Alexander Kornienko <alexfh at google.com>
>>> wrote:
>>>
>>>> alexfh added inline comments.
>>>>
>>>> ================
>>>> Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119
>>>> @@ -115,1 +118,3 @@
>>>>      StringRef EscapedCommandLine) {
>>>> +#if defined(LLVM_ON_WIN32)
>>>> +  llvm::BumpPtrAllocator Alloc;
>>>> ----------------
>>>> As noted on D23409, this will likely break mingw compatibility in clang
>>>> on windows. We should either add a sort of auto-detection of the command
>>>> line format, or extend the JSON compilation database with a way to specify
>>>> the command line format used (or as Reid suggests, specify a list of
>>>> arguments, but this should be done in a backward-compatible way).
>>>>
>>>>
>>>> https://reviews.llvm.org/D23455
>>>>
>>>>
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160814/e3872711/attachment.html>