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 <ztur...@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 <kli...@google.com> wrote: > >> >> >> On Sat, Aug 13, 2016, 10:17 PM Zachary Turner <ztur...@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 <ale...@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 >>>> >>>> >>>> >>>>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits