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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to