[PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 26 06:47:51 PDT 2016


hokein added inline comments.

================
Comment at: clang-tidy/ClangTidyOptions.cpp:235
@@ +234,3 @@
+    if (Iter != CachedOptions.end()) {
+      RawOptions.push_back(Iter->second);
+      break;
----------------
alexfh wrote:
> This seems to be changing the caching logic. Consider this directory structure:
> 
>   a/
>      .clang-tidy
>      b/
>         c/
> 
> And consequtive `getRawOptions` calls with:
>   1. "a/some_file"
>   2. "a/b/c/some_file"
>   3. "a/b/some_file".
> 
> What would happen previously:
>   1. after call 1 `CachedOptions` would contain an entry for "a"
>   2. call 2 would find an entry for "a" and copy it for "a/b" and "a/b/c"
>   3. call 3 would just use the cache entry for "a/b"
> 
> Now step 2 doesn't copy the cache entry to "a/b" and "a/b/c".
> 
> Is there any specific reason to change this? This is benign given that the lookups happen in memory, but then the code needs to be consistent and avoid replicating cache entries to intermediate directories in all cases.
Oh, I add a `break` statement here accidently. Remove it, and keep the caching logic here now. 

================
Comment at: clang-tidy/ClangTidyOptions.h:115
@@ +114,3 @@
+  ///    * clang-tidy binary
+  ///    * '-config' commandline option or a specific configuration file
+  ///    * '-checks' commandline option
----------------
Explaining the priority of `config` option and config file is't reasonable here since clang-tidy only takes one of them. If the config option is specified, clang-tidy just ignores the config file.


http://reviews.llvm.org/D18694





More information about the cfe-commits mailing list