[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/tool/clang-tidy-diff.py:51
+
+      if not timeout is None:
+        watchdog = threading.Timer(timeout, proc.kill)
----------------
`if timeout is not None` is more readable.


================
Comment at: clang-tidy/tool/clang-tidy-diff.py:58
+      with lock:
+        sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + '\n')
+        if stderr:
----------------
the `command` could be utf-8, too not?
If for example file-names use special characters they should be decoded as well. This is probably not done properly in the `run-clang-tidy.py` script, but can be adressed separatly.


================
Comment at: clang-tidy/tool/clang-tidy-diff.py:76
+    t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+    t.daemon = True
+    t.start()
----------------
why daemonize? Who kills that thread in the end? I think it's not the best choice.


================
Comment at: clang-tidy/tool/clang-tidy-diff.py:131
 
+  if args.j == 0 or args.j > 1:
+    if args.export_fixes:
----------------
I would prefer going the `clang-apply-replacements` route here as well.
It feels weird if there is an inconsistency between these parallel runners.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57662/new/

https://reviews.llvm.org/D57662