[PATCH] D54257: [clang-tidy] **Prototype** use AllTUsExecutors

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 8 15:19:45 PST 2018


JonasToth added a comment.

Very interesting. As I am not very familiar with all the internals I do have a few questions :)

Right now notes seem not be closely attached to their related warning. But within the there is a check, that a note is emitted after an error. Would it make sense to group these errors and notes together?
How would the dedup happen for static-analyzer diagnostics? Right now its only from the source-location, but as mentionend in the other review there might be many paths that lead to the same diagnostics, with the difference in the notes.

Did you check with TSAN if you get some hints on what could be the problem on parallel execution? Would you do the synchronization within `ClangTidyContext` or are there other places as well that lead to race-conditions?



================
Comment at: clang-tidy/ClangTidy.cpp:566
+      std::sort(Results.begin(), Results.end());
+      Results.erase(std::unique(Results.begin(), Results.end()), Results.end());
+    }
----------------
is this deduplication, or the other place (or both? and if yes why at two places?)


================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:652
+  std::sort(Errors.begin(), Errors.end());
+  Errors.erase(std::unique(Errors.begin(), Errors.end()), Errors.end());
   if (RemoveIncompatibleErrors)
----------------
Or this dedup?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54257





More information about the cfe-commits mailing list