[PATCH] Fix JSON compilation database command unescaping.

[Manuel Klimek]

Also, if you want to update the patch, you don't need to abandon the
revision - if you abandon the revision there is not much more you can do
with it... (and I cannot patch it in and see what you actually are trying
to fix).

The general idea behind the unescaping is that we want to be able to put
shell commands "as if you wanted to pass them to shell" into the "command"
entry...

On Mon Nov 24 2014 at 11:03:24 AM Manuel Klimek <klimek at google.com> wrote:

> On Mon Nov 24 2014 at 10:57:12 AM Daniel Cheng <dcheng at google.com> wrote:
>
>> I abandoned the previous diff, because I was flailing about the interface
>> and had no idea what I was doing.
>>
>> As for the fix, the JSON compilation database spec (
>> http://clang.llvm.org/docs/JSONCompilationDatabase.html) says:
>> > Parameters use shell quoting and shell escaping of quotes.
>>
>> The problem is that `skipEscapeCharacter()` always skips over a backslash
>> character, instead of the behavior described in the spec (only shell
>> escaping of quotes). Ninja and CMake expect that shell escaping is only
>> applied to quotes, and output compile database commands like:
>> ```"clang-cl.exe path\\to\\input.cc"```
>>
>> After JSON unescaping, the command looks like:
>> ```clang-cl.exe path\to\input.cc```
>>
>> But because the argument parser always skips backslashes,
>> `JSONCompilationDatabase::getCompileCommands` ends up returning:
>> ```clang-cl.exe pathtoinput.cc```
>>
>> Which obviously doesn't work. One alternative is to decide that all
>> normal shell escapes apply and change the spec. This has one big
>> disadvantage: the JSON decoder already turns escape sequences into their
>> special character (e.g. `\n` to an actual newline character). Why does the
>> argument unescaper need to do the same thing?
>>
>> Finally, there were no tests for the command argument unescaping
>> behavior, so I modified `JSONCompilationDatabase.GetAl
>
>
> What about these?
>
> http://reviews.llvm.org/diffusion/L/browse/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp;222646$294
>
>
>
>> lCompileCommands` to cover several argument parser behaviors:
>> - Double quotes allow spaces to be included in a single argument.
>> - Escaped quotes are skipped over inside a double quoted argument.
>> - Backslashes are included in the unescaped argument if they weren't
>> escaping a quote.
>> - Backslashes are allowed as the last character of a single argument.
>>
>> http://reviews.llvm.org/D6376
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141124/7d8f6ecf/attachment.html>