[clang-tidy] Add exit code support to clang-tidy-diff.py

[PiotrZSL]

Summary

Modify script to fail when some run clang-tidy
command fails. Based on run_clang-tidy.

Fixes: #65000

[FlashSheridan]

Thank you, looks promising and I will test it on Monday.

[FlashSheridan]

A couple of low-priority suggestions from Pylint 3 at 326 and 95:

clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py

95

Pylint 3 says > “[R0913(too-many-arguments), start_workers] Too many arguments (6/5),”

I think you just pushed start_workers() over the edge; personally, I don’t think a rewrite to reduce the number of arguments would improve clarity.

326

Pylint 3 says: > [C1802(use-implicit-booleaness-not-len), main] Do not use len(SEQUENCE) without comparison to determine if a sequence is empty

(I have no strong feelings.)

[PiotrZSL]

Fix some Pylint issues.

[FlashSheridan]

Thank you kindly, this is looking very good on our recent changes in my manual usage. (It will take some more work before I can run it in our GitHub Action, which I’m looking forward to.)

[carlosgalvezp]

LGTM!

clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py

 is_py2 = sys.version[0] == "2"
 
 if is_py2:
     import Queue as queue
 else:
     import queue as queue
 
 
-def run_tidy(task_queue, lock, timeout):
+def run_tidy(task_queue, lock, timeout, failed_files):
     watchdog = None
     while True:
         command = task_queue.get()
         try:
             proc = subprocess.Popen(
                 command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
             )
 
             if timeout is not None:
                 watchdog = threading.Timer(timeout, proc.kill)
                 watchdog.start()
 
             stdout, stderr = proc.communicate()
+            if proc.returncode != 0:
+                if proc.returncode < 0:
+                    msg = "Terminated by signal %d : %s\n" % (
+                        -proc.returncode,
+                        " ".join(command),
+                    )
+                    stderr += msg.encode("utf-8")
+                failed_files.append(command)
 
             with lock:
                 sys.stdout.write(stdout.decode("utf-8") + "\n")
                 sys.stdout.flush()
                 if stderr:
                     sys.stderr.write(stderr.decode("utf-8") + "\n")
                     sys.stderr.flush()
         except Exception as e:
             with lock:
                 sys.stderr.write("Failed: " + str(e) + ": ".join(command) + "\n")
         finally:
             with lock:
                 if not (timeout is None or watchdog is None):
                     if not watchdog.is_alive():
                         sys.stderr.write(
                             "Terminated by timeout: " + " ".join(command) + "\n"
                         )
                     watchdog.cancel()
             task_queue.task_done()
 
 
-def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout, failed_files):
     for _ in range(max_tasks):
-        t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+        t = threading.Thread(
+            target=tidy_caller, args=(task_queue, lock, timeout, failed_files)
+        )
         t.daemon = True
         t.start()
 
 
 def merge_replacement_files(tmpdir, mergefile):
     """Merge all replacement files in a directory into a single file"""
     # The fixes suggested by clang-tidy >= 4.0.0 are given under
     # the top level key 'Diagnostics' in the output yaml files
def main():
     if yaml and args.export_fixes:
         tmpdir = tempfile.mkdtemp()
 
     # Tasks for clang-tidy.
     task_queue = queue.Queue(max_task_count)
     # A lock for console output.
     lock = threading.Lock()
 
+    # List of files with a non-zero return code.
+    failed_files = []
+
     # Run a pool of clang-tidy workers.
-    start_workers(max_task_count, run_tidy, task_queue, lock, args.timeout)
+    start_workers(
+        max_task_count, run_tidy, task_queue, lock, args.timeout, failed_files
+    )
 
     # Form the common args list.
     common_clang_tidy_args = []
     if args.fix:
         common_clang_tidy_args.append("-fix")
     if args.checks != "":
         common_clang_tidy_args.append("-checks=" + args.checks)
     if args.quiet:
for name in lines_by_file:
             os.close(handle)
             command.append("-export-fixes=" + tmp_name)
         command.extend(common_clang_tidy_args)
         command.append(name)
         command.extend(clang_tidy_args)
 
         task_queue.put(command)
 
+    # Application return code
+    return_code = 0
+
     # Wait for all threads to be done.
     task_queue.join()
+    # Application return code
+    return_code = 0
+    if len(failed_files):
+        return_code = 1
 
     if yaml and args.export_fixes:
         print("Writing fixes to " + args.export_fixes + " ...")
         try:
             merge_replacement_files(tmpdir, args.export_fixes)
         except:
             sys.stderr.write("Error exporting fixes.\n")
             traceback.print_exc()
+            return_code = 1
 
     if tmpdir:
         shutil.rmtree(tmpdir)
+    sys.exit(return_code)
 
 
 if __name__ == "__main__":
     main()

clang-tools-extra/docs/ReleaseNotes.rst

- Preprocessor-level module header parsing is now disabled by default due to
   functionality using the newly added command line option
   `--enable-module-headers-parsing`.
 
 - Remove configuration option `AnalyzeTemporaryDestructors`, which was deprecated since
   :program:`clang-tidy` 16.
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1`
+  if any :program:`clang-tidy` subprocess exits with a non-zero code or if
+  exporting fixes fails.
+
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-inc-dec-in-conditions
   <clang-tidy/checks/bugprone/inc-dec-in-conditions>` check.
 
   Detects when a variable is both incremented/decremented and referenced inside
   a complex condition and suggests moving them outside to avoid ambiguity in