[PATCH] D23409: Make clang-tidy work with clang-cl

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 11 13:55:54 PDT 2016


alexfh added a comment.

May I ask you to upload patches with full diff context next time? I know, it's not directly supported by TortoiseGit, but there are at least two other reasonable ways of generating full diffs for Phabricator: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface and http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.

Wrt testing, I think, apart from a new clang-tidy test, the changes to Driver and Tooling need separate tests as well.


================
Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119
@@ -115,1 +118,3 @@
     StringRef EscapedCommandLine) {
+#if defined(LLVM_ON_WIN32)
+  llvm::BumpPtrAllocator Alloc;
----------------
zturner wrote:
> rnk wrote:
> > It would be nice if the JSON file just told us which quoting mechanism it was using. You can imagine building the compilation database on one system and sending it off to another for indexing.
> Correct me if I've got this wrong but:
> 
> 1. If you had a compile command database generated on non-Windows, and you used it on Windows, then it would necessarily be referring to something other than clang-cl right?  In which case, we don't support running clang in non-cl mode on Windows.
> 
> 2. If you had a compile command database generated on Windows, then the only supported configuration would involve clang-cl, in which case you couldn't use it on non-Windows.
> 
> Is there a supported use case where a non-Windows compile database would be used on Windows or vice versa?
> we don't support running clang in non-cl mode on Windows.

I think, clang in non-cl mode is functional on Windows to a certain degree (mingw-compatibility or something like this). Artificially preventing it from working doesn't seem to be right.


https://reviews.llvm.org/D23409





More information about the cfe-commits mailing list