Page MenuHomePhabricator

[clang-tidy] **Prototype** use AllTUsExecutors
Needs ReviewPublic

Authored by hokein on Nov 8 2018, 6:34 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Only works on StandaloneToolExecutor, crashes happen when running
AllTUsExecutors with multithreads (I believe it is because
ClangTidyContext is not threadsafe).

// Restrict thread to 1.
./bin/clang-tidy -executor=all-TUs -filter=".*/extra/*" -checks="-*,readability-braces-around-statements" -execute-concurrency=1 -export-fixes="/tmp/fixes.yaml" .

Event Timeline

hokein created this revision.Nov 8 2018, 6:34 AM

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?

clang-tidy/ClangTidy.cpp
566

is this deduplication, or the other place (or both? and if yes why at two places?)

clang-tidy/ClangTidyDiagnosticConsumer.cpp
652

Or this dedup?