[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
@@ -501,70 +506,72 @@ def main(): # Build up a big regexy filter from all command line arguments. file_name_re = re.compile("|".join(args.files)) +files = {f for f in files if file_name_re.search(f)} -return_code = 0 +returncode = 0 try: -# Spin up a bunch of tidy-launching threads. -task_queue = queue.Queue(max_task) -# List of files with a non-zero return code. -failed_files = [] -lock = threading.Lock() -for _ in range(max_task): -t = threading.Thread( -target=run_tidy, -args=( -args, -clang_tidy_binary, -export_fixes_dir, -build_path, -task_queue, -lock, -failed_files, -), +semaphore = asyncio.Semaphore(max_task) +tasks = [ +run_with_semaphore( +semaphore, +run_tidy, +args, +f, +clang_tidy_binary, +export_fixes_dir, +build_path, ) -t.daemon = True -t.start() - -# Fill the queue with files. -for name in files: -if file_name_re.search(name): -task_queue.put(name) - -# Wait for all threads to be done. -task_queue.join() -if len(failed_files): -return_code = 1 - +for f in files +] + +for i, coro in enumerate(asyncio.as_completed(tasks)): +name, process_returncode, stdout, stderr = await coro +if process_returncode != 0: +returncode = 1 +if process_returncode < 0: +stderr += f"{name}: terminated by signal {-process_returncode}\n" +print(f"[{i + 1}/{len(files)}] {name}") +if stdout: +print(stdout) +if stderr: +print(stderr, file=sys.stderr) except KeyboardInterrupt: # This is a sad hack. Unfortunately subprocess goes # bonkers with ctrl-c and we start forking merrily. print("\nCtrl-C detected, goodbye.") if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) os.kill(0, 9) if combine_fixes: -print("Writing fixes to " + args.export_fixes + " ...") +print(f"Writing fixes to {args.export_fixes} ...") try: +assert export_fixes_dir merge_replacement_files(export_fixes_dir, args.export_fixes) except: print("Error exporting fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if args.fix: print("Applying fixes ...") try: +assert export_fixes_dir apply_fixes(args, clang_apply_replacements_binary, export_fixes_dir) except: print("Error applying fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) -sys.exit(return_code) +sys.exit(returncode) if __name__ == "__main__": -main() +# FIXME Python 3.7: This can be simplified by asyncio.run(main()). +loop = asyncio.new_event_loop() +loop.run_until_complete(main()) +loop.close() 5chmidti wrote: Sure, but why does `main` need to be `async` at all? I.e. ```python async def main(): pass if __name__ == "__main__": # FIXME Python 3.7: This can be simplified by asyncio.run(main()). loop = asyncio.new_event_loop() loop.run_until_complete(main()) loop.close() ``` vs. ```python def main(): pass if __name__ == "__main__": main() ``` https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/5chmidti commented: The execution time of clang-tidy is a nice improvement as well. --- > What OS/Python version does this occur on? |OS|Python|Noisy Shutdown| |-|-|-| |Manjaro|3.11.9|Yes| |NixOS|3.9.19|No| |NixOS|3.10.14|No| |NixOS|3.11.9|No| |Manjaro|3.12.3|Yes| |Manjaro|3.13.0b2|Yes| https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
nicovank wrote: - Minor change to progress indicator to stay aligned. - Go back to printing command line. I'm fine with leaving this as-is. - I think I pruned some changes by accident on rebase, thanks @5chmidti! Restored runtime number and initial message. -- > More graceful shutdown would be welcome. @PiotrZSL I can't reproduce on my setup (MacOS/3.12). What OS/Python version does this occur on? Will look into better shutdown. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From 33485bf2ab3c683897f1381f3f4ab8294bdb0b72 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 314 ++ clang-tools-extra/docs/ReleaseNotes.rst | 2 + 2 files changed, 182 insertions(+), 134 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 4dd20bec81d3b..106632428d131 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,31 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading +import time import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: yaml = None -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +69,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,48 +85,42 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -exclude_header_filter, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +exclude_header_filter: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: start.append("-allow-enabling-analyzer-alpha-checkers") if exclude_header_filter is not None: -start.append("--exclude-header-filter=" + exclude_header_filter) +start.append(f"--exclude-header-filter={exclude_header_filter}") if header_filter is not None: -start.append("-header-filter=" + header_filter) +start.append(f"-header-filter={header_filter}") if line_filter is not None: -start.append("-line-filter=" + line_filter) +start.append(f"-line-filter={line_filter}") if use_color is not None: if use_color: start.append("--use-color") else: start.append("--use-color=false") if checks: -start.append("-checks=" + checks) +start.append(f"-checks={checks}") if tmpdir is not None: start.append("-export-fixes") # Get a temporary file. We immediately close the handle so clang-tidy can @@ -133,26 +129,27 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) -start.append("-p=" + build_path) +start.append(f"-extra-arg-before={arg}") +start.append(f"-p={build_path}") if quiet: start.append("-quiet") if config_file_path: -start.append("--config-file=" + config_file_path) +start.append(f"--config-file={config_file_path}") elif config: -start.append("-config=" + config) +start.append(f"-config={config}") for plugin in plugins:
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
@@ -501,70 +506,72 @@ def main(): # Build up a big regexy filter from all command line arguments. file_name_re = re.compile("|".join(args.files)) +files = {f for f in files if file_name_re.search(f)} -return_code = 0 +returncode = 0 try: -# Spin up a bunch of tidy-launching threads. -task_queue = queue.Queue(max_task) -# List of files with a non-zero return code. -failed_files = [] -lock = threading.Lock() -for _ in range(max_task): -t = threading.Thread( -target=run_tidy, -args=( -args, -clang_tidy_binary, -export_fixes_dir, -build_path, -task_queue, -lock, -failed_files, -), +semaphore = asyncio.Semaphore(max_task) +tasks = [ +run_with_semaphore( +semaphore, +run_tidy, +args, +f, +clang_tidy_binary, +export_fixes_dir, +build_path, ) -t.daemon = True -t.start() - -# Fill the queue with files. -for name in files: -if file_name_re.search(name): -task_queue.put(name) - -# Wait for all threads to be done. -task_queue.join() -if len(failed_files): -return_code = 1 - +for f in files +] + +for i, coro in enumerate(asyncio.as_completed(tasks)): +name, process_returncode, stdout, stderr = await coro +if process_returncode != 0: +returncode = 1 +if process_returncode < 0: +stderr += f"{name}: terminated by signal {-process_returncode}\n" +print(f"[{i + 1}/{len(files)}] {name}") +if stdout: +print(stdout) +if stderr: +print(stderr, file=sys.stderr) except KeyboardInterrupt: # This is a sad hack. Unfortunately subprocess goes # bonkers with ctrl-c and we start forking merrily. print("\nCtrl-C detected, goodbye.") if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) os.kill(0, 9) if combine_fixes: -print("Writing fixes to " + args.export_fixes + " ...") +print(f"Writing fixes to {args.export_fixes} ...") try: +assert export_fixes_dir merge_replacement_files(export_fixes_dir, args.export_fixes) except: print("Error exporting fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if args.fix: print("Applying fixes ...") try: +assert export_fixes_dir apply_fixes(args, clang_apply_replacements_binary, export_fixes_dir) except: print("Error applying fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) -sys.exit(return_code) +sys.exit(returncode) if __name__ == "__main__": -main() +# FIXME Python 3.7: This can be simplified by asyncio.run(main()). +loop = asyncio.new_event_loop() +loop.run_until_complete(main()) +loop.close() nicovank wrote: If you call main or any other `async` function without special handling you get some ``` RuntimeWarning: coroutine 'main' was never awaited ``` Since `await` can only be used in `async` functions, it's common to use [`asyncio.run`](https://docs.python.org/3/library/asyncio-runner.html#asyncio.run) and wrap `main`. That is Python 3.7 and above, we get compatibility with 3.6 by using those 3 lines instead. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/5chmidti commented: Thanks for the cleanup, it looks overall good (w.r.t `asyncio`: I only know about `asyncio` what I read in this pr). > Only print the filename after completion, not the entire Clang-Tidy > invocation command. I find this neater but the behavior can easily be > restored. > By default print like in original script (command line), but add --progress > switch that will change it to current behavior. I'd prefer the original behavior as well, but printing only the filename can be added as a flag. Although, I'm not sure `--progress` is a good name for something that only changes if the command or file is printed, because the progress indicator should be emitted either way. > Added an initial message with the number of files to process over the number > of files in compilation database. Maybe that was removed? Either way, I think it's +- not necessary, but I'm not against having it. > More graceful shutdown would be welcome. +1 IMO, that needs to be done before merging. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
@@ -501,70 +506,72 @@ def main(): # Build up a big regexy filter from all command line arguments. file_name_re = re.compile("|".join(args.files)) +files = {f for f in files if file_name_re.search(f)} -return_code = 0 +returncode = 0 try: -# Spin up a bunch of tidy-launching threads. -task_queue = queue.Queue(max_task) -# List of files with a non-zero return code. -failed_files = [] -lock = threading.Lock() -for _ in range(max_task): -t = threading.Thread( -target=run_tidy, -args=( -args, -clang_tidy_binary, -export_fixes_dir, -build_path, -task_queue, -lock, -failed_files, -), +semaphore = asyncio.Semaphore(max_task) +tasks = [ +run_with_semaphore( +semaphore, +run_tidy, +args, +f, +clang_tidy_binary, +export_fixes_dir, +build_path, ) -t.daemon = True -t.start() - -# Fill the queue with files. -for name in files: -if file_name_re.search(name): -task_queue.put(name) - -# Wait for all threads to be done. -task_queue.join() -if len(failed_files): -return_code = 1 - +for f in files +] + +for i, coro in enumerate(asyncio.as_completed(tasks)): +name, process_returncode, stdout, stderr = await coro +if process_returncode != 0: +returncode = 1 +if process_returncode < 0: +stderr += f"{name}: terminated by signal {-process_returncode}\n" +print(f"[{i + 1}/{len(files)}] {name}") 5chmidti wrote: It would look even nicer if the first number was printed with the width of the second number, i.e.: ``` [ 3/15] ... [15/15] ``` That way, the position of the second number and the command(/file) does not shift when the next power of ten is reached. ```python i = 4 num_files = 15 print("[{0: >{1}}/{2}]".format(i+1,len(f"{num_files}"), num_files)) print(f'[{i+1: >{len(f"{num_files}")}}/{num_files}]') ``` https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
@@ -501,70 +506,72 @@ def main(): # Build up a big regexy filter from all command line arguments. file_name_re = re.compile("|".join(args.files)) +files = {f for f in files if file_name_re.search(f)} -return_code = 0 +returncode = 0 try: -# Spin up a bunch of tidy-launching threads. -task_queue = queue.Queue(max_task) -# List of files with a non-zero return code. -failed_files = [] -lock = threading.Lock() -for _ in range(max_task): -t = threading.Thread( -target=run_tidy, -args=( -args, -clang_tidy_binary, -export_fixes_dir, -build_path, -task_queue, -lock, -failed_files, -), +semaphore = asyncio.Semaphore(max_task) +tasks = [ +run_with_semaphore( +semaphore, +run_tidy, +args, +f, +clang_tidy_binary, +export_fixes_dir, +build_path, ) -t.daemon = True -t.start() - -# Fill the queue with files. -for name in files: -if file_name_re.search(name): -task_queue.put(name) - -# Wait for all threads to be done. -task_queue.join() -if len(failed_files): -return_code = 1 - +for f in files +] + +for i, coro in enumerate(asyncio.as_completed(tasks)): +name, process_returncode, stdout, stderr = await coro +if process_returncode != 0: +returncode = 1 +if process_returncode < 0: +stderr += f"{name}: terminated by signal {-process_returncode}\n" +print(f"[{i + 1}/{len(files)}] {name}") +if stdout: +print(stdout) +if stderr: +print(stderr, file=sys.stderr) except KeyboardInterrupt: # This is a sad hack. Unfortunately subprocess goes # bonkers with ctrl-c and we start forking merrily. print("\nCtrl-C detected, goodbye.") if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) os.kill(0, 9) if combine_fixes: -print("Writing fixes to " + args.export_fixes + " ...") +print(f"Writing fixes to {args.export_fixes} ...") try: +assert export_fixes_dir merge_replacement_files(export_fixes_dir, args.export_fixes) except: print("Error exporting fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if args.fix: print("Applying fixes ...") try: +assert export_fixes_dir apply_fixes(args, clang_apply_replacements_binary, export_fixes_dir) except: print("Error applying fixes.\n", file=sys.stderr) traceback.print_exc() -return_code = 1 +returncode = 1 if delete_fixes_dir: +assert export_fixes_dir shutil.rmtree(export_fixes_dir) -sys.exit(return_code) +sys.exit(returncode) if __name__ == "__main__": -main() +# FIXME Python 3.7: This can be simplified by asyncio.run(main()). +loop = asyncio.new_event_loop() +loop.run_until_complete(main()) +loop.close() 5chmidti wrote: Just asking: Being unfamiliar with `asyncio`, why do we need this part and an `async main`, instead of calling plain `main`? https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
PiotrZSL wrote: @nicovank Overall looks fine from functionality point of view. Problem A) Getting bunch of ``` task: wait_for= cb=[as_completed.._on_completion() at /usr/lib/python3.12/asyncio/tasks.py:618]> Task was destroyed but it is pending! ``` When CTR+C a script. More graceful shutdown would be welcome. Problem B) Previously script printed clang-tidy command line, now just progress + filename, for me it's fine but some users could find this as an degradation, as previously you could simply copy command line output and just run it manually. My advice: - By default print like in original script (command line), but add --progress switch that will change it to current behavior. - Add release notes entry about updates in script that impact end user. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
PiotrZSL wrote: @nicovank I think that code is fine, will re-check it today. We could always think about something else later. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
nicovank wrote: Rebased on top of cc54129b983799e1aaea77aa0ff3040dc30cbc8c. Ping @PiotrZSL, what to do here? I can keep the current flush and separate stdout/stderr behavior in this PR, it can be changed later. But I don't think flushing fixes that issue of mis-ordered output lines, IMO either 1. use the same stream to force order 2. let user unbuffer with the `python3 -u` flag. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From da3ec0033706609c79216fd2e5360c49304223f4 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 275 +- 1 file changed, 141 insertions(+), 134 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 4dd20bec81d3b..327e2dca6160d 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: yaml = None -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,48 +84,42 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -exclude_header_filter, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +exclude_header_filter: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: start.append("-allow-enabling-analyzer-alpha-checkers") if exclude_header_filter is not None: -start.append("--exclude-header-filter=" + exclude_header_filter) +start.append(f"--exclude-header-filter={exclude_header_filter}") if header_filter is not None: -start.append("-header-filter=" + header_filter) +start.append(f"-header-filter={header_filter}") if line_filter is not None: -start.append("-line-filter=" + line_filter) +start.append(f"-line-filter={line_filter}") if use_color is not None: if use_color: start.append("--use-color") else: start.append("--use-color=false") if checks: -start.append("-checks=" + checks) +start.append(f"-checks={checks}") if tmpdir is not None: start.append("-export-fixes") # Get a temporary file. We immediately close the handle so clang-tidy can @@ -133,26 +128,27 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) -start.append("-p=" + build_path) +start.append(f"-extra-arg-before={arg}") +start.append(f"-p={build_path}") if quiet: start.append("-quiet") if config_file_path: -start.append("--config-file=" + config_file_path) +start.append(f"--config-file={config_file_path}") elif config: -start.append("-config=" + config) +start.append(f"-config={config}") for plugin in plugins: -start.append("-load=" + plugin) +
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From 129187f336bf6351dae4604d690809f4095a2e7e Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 274 ++ 1 file changed, 152 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..2a00d29e0b93de 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,31 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading +import time import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +69,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +85,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: @@ -130,9 +126,9 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) +start.append(f"-extra-arg-before={arg}") start.append("-p=" + build_path) if quiet: start.append("-quiet") @@ -148,8 +144,9 @@ def get_tidy_invocation( return start -def merge_replacement_files(tmpdir, mergefile): +def merge_replacement_files(tmpdir: str, mergefile: str) -> None: """Merge all replacement files in a directory into a single file""" +assert yaml # The fixes suggested by clang-tidy >= 4.0.0 are given under # the top level key 'Diagnostics' in the output yaml files mergekey = "Diagnostics" @@ -173,16 +170,14 @@ def merge_replacement_files(tmpdir, mergefile): open(mergefile, "w").close() -def find_binary(arg, name, build_path): +def find_binary(arg: str, name: str, build_path: str) -> str: """Get the path for a binary or exit""" if arg: if shutil.which(arg): return arg else: raise SystemExit( -"error: passed binary '{}' was not found or is not executable".format( -arg -) +f"error: passed binary '{arg}' was not found or is not executable" ) built_path = os.path.join(build_path, "bin", name) @@ -190,12 +185,12 @@ def find_binary(arg, name, build_path): if binary: return binary else: -raise SystemExit( -"error: failed to find {} in $PATH or at {}".format(name, built_path) -) +raise SystemExit(f"error: failed to find
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
nicovank wrote: Reproduced the buffering issue on my setup, but flushing didn't fix the problem. I could also reproduce with the current version of this script, which does also flush after writing. Python defaults input/output streams to buffered mode when it detects a pipe, and I don't think flushing overrides that setting. One way to force unbuffering I know of is to run `python3 -u` when initially invoking the script, but that's not great here. Reproduction with current script version. ``` % python3 run-clang-tidy.py -checks="-*,performance-*" -source-filter ".*clang-tools-extra/clang-query.*" &>output /home/nvankempen_umass_edu/work/build/bin/clang-tidy -checks=-*,performance-* -p=/home/nvankempen_umass_edu/work/llvm-project /home/nvankempen_umass_edu/work/llvm-project/clang-tools-extra/clang-query/QueryParser.cpp /home/nvankempen_umass_edu/work/llvm-project/clang-tools-extra/clang-query/QueryParser.cpp:175:6: warning: enum 'ParsedQueryKind' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size] 175 | enum ParsedQueryKind { | ^ /home/nvankempen_umass_edu/work/llvm-project/clang-tools-extra/clang-query/QueryParser.cpp:189:6: warning: enum 'ParsedQueryVariable' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size] 189 | enum ParsedQueryVariable { | ^ /home/nvankempen_umass_edu/work/build/bin/clang-tidy -checks=-*,performance-* -p=/home/nvankempen_umass_edu/work/llvm-project /home/nvankempen_umass_edu/work/llvm-project/clang-tools-extra/clang-query/tool/ClangQuery.cpp /home/nvankempen_umass_edu/work/build/bin/clang-tidy -checks=-*,performance-* -p=/home/nvankempen_umass_edu/work/llvm-project /home/nvankempen_umass_edu/work/llvm-project/clang-tools-extra/clang-query/Query.cpp 1363 warnings generated. Suppressed 1361 warnings (1361 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1595 warnings generated. Suppressed 1595 warnings (1595 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1633 warnings generated. Suppressed 1633 warnings (1633 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. ``` I think the way to go here is just to print everything to stdout, stderr can be reserved to issues within the script itself. **→ This is the behavior in the current version of this PR**. Added an initial message with the number of files to process over the number of files in compilation database. Added a runtime number for each file. Thought about making a `ClangTidyResult` class to wrap the `run_tidy` return, but it's only used once and would make the deconstructing harder, so it's fine. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From 85700b3488ceb65adc73469c82137c1c3429b3f4 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 274 ++ 1 file changed, 152 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..1ebbadcb5005b8 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,31 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading +import time import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +69,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +85,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: @@ -130,9 +126,9 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) +start.append(f"-extra-arg-before={arg}") start.append("-p=" + build_path) if quiet: start.append("-quiet") @@ -148,8 +144,9 @@ def get_tidy_invocation( return start -def merge_replacement_files(tmpdir, mergefile): +def merge_replacement_files(tmpdir: str, mergefile: str) -> None: """Merge all replacement files in a directory into a single file""" +assert yaml # The fixes suggested by clang-tidy >= 4.0.0 are given under # the top level key 'Diagnostics' in the output yaml files mergekey = "Diagnostics" @@ -173,16 +170,14 @@ def merge_replacement_files(tmpdir, mergefile): open(mergefile, "w").close() -def find_binary(arg, name, build_path): +def find_binary(arg: str, name: str, build_path: str) -> str: """Get the path for a binary or exit""" if arg: if shutil.which(arg): return arg else: raise SystemExit( -"error: passed binary '{}' was not found or is not executable".format( -arg -) +f"error: passed binary '{arg}' was not found or is not executable" ) built_path = os.path.join(build_path, "bin", name) @@ -190,12 +185,12 @@ def find_binary(arg, name, build_path): if binary: return binary else: -raise SystemExit( -"error: failed to find {} in $PATH or at {}".format(name, built_path) -) +raise SystemExit(f"error: failed to find
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
@@ -488,70 +493,72 @@ def main(): # Build up a big regexy filter from all command line arguments. file_name_re = re.compile("|".join(args.files)) +files = {f for f in files if file_name_re.search(f)} -return_code = 0 +returncode = 0 try: -# Spin up a bunch of tidy-launching threads. -task_queue = queue.Queue(max_task) -# List of files with a non-zero return code. -failed_files = [] -lock = threading.Lock() -for _ in range(max_task): -t = threading.Thread( -target=run_tidy, -args=( -args, -clang_tidy_binary, -export_fixes_dir, -build_path, -task_queue, -lock, -failed_files, -), +semaphore = asyncio.Semaphore(max_task) +tasks = [ +run_with_semaphore( +semaphore, +run_tidy, +args, +f, +clang_tidy_binary, +export_fixes_dir, +build_path, ) -t.daemon = True -t.start() - -# Fill the queue with files. -for name in files: -if file_name_re.search(name): -task_queue.put(name) - -# Wait for all threads to be done. -task_queue.join() -if len(failed_files): -return_code = 1 - +for f in files +] + +for i, coro in enumerate(asyncio.as_completed(tasks)): +name, process_returncode, stdout, stderr = await coro +if process_returncode != 0: +returncode = 1 +if process_returncode < 0: +stderr += f"{name}: terminated by signal {-process_returncode}\n" +print(f"[{i + 1}/{len(files)}] {name}") +if stdout: +print(stdout) +if stderr: +print(stderr, file=sys.stderr) PiotrZSL wrote: Make sure to add flush to both buffers after printing. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/PiotrZSL commented: One issue, looks like only stdout is buffered, and when clang-tidy output: "26035 warnings generated. Suppressed 26030 warnings (26030 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 26005 warnings generated. Suppressed 26002 warnings (26002 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 25999 warnings generated. Suppressed 25996 warnings (25996 in non-user code). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. " it's just displayed without point to an file that produced this output. This is what I see when redirecting output to file with "&> output.txt". Looks like without redirection to file works fine. Second issue, is that it takes a while before first output is being shown to user. Would be nice if we would show something like "Running clang-tidy for XYZ files out of ZXY in compilation database", or something like this. You may consider adding also an timer how long command took for every file. Could be usefull https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From e27931b8a77c49f1c4cd47ca076addbe0ed19d87 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 251 +- 1 file changed, 129 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..7ec86c7aada593 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +84,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: @@ -130,9 +125,9 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) +start.append(f"-extra-arg-before={arg}") start.append("-p=" + build_path) if quiet: start.append("-quiet") @@ -148,8 +143,9 @@ def get_tidy_invocation( return start -def merge_replacement_files(tmpdir, mergefile): +def merge_replacement_files(tmpdir: str, mergefile: str) -> None: """Merge all replacement files in a directory into a single file""" +assert yaml # The fixes suggested by clang-tidy >= 4.0.0 are given under # the top level key 'Diagnostics' in the output yaml files mergekey = "Diagnostics" @@ -173,16 +169,14 @@ def merge_replacement_files(tmpdir, mergefile): open(mergefile, "w").close() -def find_binary(arg, name, build_path): +def find_binary(arg: str, name: str, build_path: str) -> str: """Get the path for a binary or exit""" if arg: if shutil.which(arg): return arg else: raise SystemExit( -"error: passed binary '{}' was not found or is not executable".format( -arg -) +f"error: passed binary '{arg}' was not found or is not executable" ) built_path = os.path.join(build_path, "bin", name) @@ -190,12 +184,12 @@ def find_binary(arg, name, build_path): if binary: return binary else: -raise SystemExit( -"error: failed to find {} in $PATH or at {}".format(name, built_path) -) +raise SystemExit(f"error: failed to find {name} in
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
nicovank wrote: > Does it work with Python 3.6? Actually, it can with one single change. `asyncio,run` was introduced in 3.7, and can be replaced. I'll just add this change and a comment. It worked on my 3.6 setup. Let me know if you run into any issues. https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
PiotrZSL wrote: I'm more or less fine with this change, but didn't had time to test it yet. Does it work with python 3.6 ? https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
nicovank wrote: @PiotrZSL @carlosgalvezp Any thoughts on this change? https://github.com/llvm/llvm-project/pull/89490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/89490 >From 8b52200a0ccfa801471e7326859276ef9b46 Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 248 +- 1 file changed, 126 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..faab4876daa898 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +84,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: @@ -130,9 +125,9 @@ def get_tidy_invocation( os.close(handle) start.append(name) for arg in extra_arg: -start.append("-extra-arg=%s" % arg) +start.append(f"-extra-arg={arg}") for arg in extra_arg_before: -start.append("-extra-arg-before=%s" % arg) +start.append(f"-extra-arg-before={arg}") start.append("-p=" + build_path) if quiet: start.append("-quiet") @@ -148,8 +143,9 @@ def get_tidy_invocation( return start -def merge_replacement_files(tmpdir, mergefile): +def merge_replacement_files(tmpdir: str, mergefile: str) -> None: """Merge all replacement files in a directory into a single file""" +assert yaml # The fixes suggested by clang-tidy >= 4.0.0 are given under # the top level key 'Diagnostics' in the output yaml files mergekey = "Diagnostics" @@ -173,16 +169,14 @@ def merge_replacement_files(tmpdir, mergefile): open(mergefile, "w").close() -def find_binary(arg, name, build_path): +def find_binary(arg: str, name: str, build_path: str) -> str: """Get the path for a binary or exit""" if arg: if shutil.which(arg): return arg else: raise SystemExit( -"error: passed binary '{}' was not found or is not executable".format( -arg -) +f"error: passed binary '{arg}' was not found or is not executable" ) built_path = os.path.join(build_path, "bin", name) @@ -190,12 +184,12 @@ def find_binary(arg, name, build_path): if binary: return binary else: -raise SystemExit( -"error: failed to find {} in $PATH or at {}".format(name, built_path) -) +raise SystemExit(f"error: failed to find {name} in
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
llvmbot wrote: @llvm/pr-subscribers-clang-tools-extra Author: Nicolas van Kempen (nicovank) Changes [There is work](https://discourse.llvm.org/t/rfc-upgrading-llvms-minimum-required-python-version/67571) to make Python 3.8 the minimum Python version for LLVM. I edited this script because I wanted some indicator of progress while going through files. It now outputs `[XX/YYY]` with the number of processed and total files after each completion. The current version of this script is compatible downto Python 3.6 (this is PyYAML's minimum version). It would probably work with older Python 3 versions with an older PyYAML or when YAML is disabled. With the updates here, it is compatible downto Python 3.7. Python 3.7 was released June 2018. https://github.com/llvm/llvm-project/pull/89302 is also touching this file, I don't mind rebasing on top of that work if needed. ### Summary - Add type annotations. - Replace `threading` + `queue` with `asyncio`. - **Add indicator of processed files over total files**. This is what I set out to do initially. - Only print the filename after completion, not the entire Clang-Tidy invocation command. I find this neater but the behavior can easily be restored. ### MyPy ``` % python3 -m mypy --strict clang-tools-extra/clang-tidy/tool/run-clang-tidy.py Success: no issues found in 1 source file ``` ### Performance Just to make sure the `asyncio` change didn't introduce a regression, I ran the previous version and this version 5 times with the following command. ``` time python3 ~/llvm-project/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \ -clang-tidy-binary="~/llvm-project/build/bin/clang-tidy" \ -clang-apply-replacements-binary="~/llvm-project/build/bin/clang-apply-replacements" \ -checks="-*,modernize-use-starts-ends-with" \ -source-filter ".*clang-tools-extra/clang-tidy.*" \ -fix -format ``` Runtimes are identical, no performance regression detected on my setup. | Version | Time (s) | |-|| | Old | 92.0 ± 1.4 | | New | 92.2 ± 1.5 | --- Full diff: https://github.com/llvm/llvm-project/pull/89490.diff 1 Files Affected: - (modified) clang-tools-extra/clang-tidy/tool/run-clang-tidy.py (+126-122) ``diff diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..530f2425322495 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +84,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers:
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Nicolas van Kempen (nicovank) Changes [There is work](https://discourse.llvm.org/t/rfc-upgrading-llvms-minimum-required-python-version/67571) to make Python 3.8 the minimum Python version for LLVM. I edited this script because I wanted some indicator of progress while going through files. It now outputs `[XX/YYY]` with the number of processed and total files after each completion. The current version of this script is compatible downto Python 3.6 (this is PyYAML's minimum version). It would probably work with older Python 3 versions with an older PyYAML or when YAML is disabled. With the updates here, it is compatible downto Python 3.7. Python 3.7 was released June 2018. https://github.com/llvm/llvm-project/pull/89302 is also touching this file, I don't mind rebasing on top of that work if needed. ### Summary - Add type annotations. - Replace `threading` + `queue` with `asyncio`. - **Add indicator of processed files over total files**. This is what I set out to do initially. - Only print the filename after completion, not the entire Clang-Tidy invocation command. I find this neater but the behavior can easily be restored. ### MyPy ``` % python3 -m mypy --strict clang-tools-extra/clang-tidy/tool/run-clang-tidy.py Success: no issues found in 1 source file ``` ### Performance Just to make sure the `asyncio` change didn't introduce a regression, I ran the previous version and this version 5 times with the following command. ``` time python3 ~/llvm-project/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \ -clang-tidy-binary="~/llvm-project/build/bin/clang-tidy" \ -clang-apply-replacements-binary="~/llvm-project/build/bin/clang-apply-replacements" \ -checks="-*,modernize-use-starts-ends-with" \ -source-filter ".*clang-tools-extra/clang-tidy.*" \ -fix -format ``` Runtimes are identical, no performance regression detected on my setup. | Version | Time (s) | |-|| | Old | 92.0 ± 1.4 | | New | 92.2 ± 1.5 | --- Full diff: https://github.com/llvm/llvm-project/pull/89490.diff 1 Files Affected: - (modified) clang-tools-extra/clang-tidy/tool/run-clang-tidy.py (+126-122) ``diff diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..530f2425322495 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +84,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: Optional[str], +) -> List[str]: """Gets a command line for clang-tidy.""" start = [clang_tidy_binary] if allow_enabling_alpha_checkers: @@
[clang-tools-extra] [run-clang-tidy.py] Refactor, add progress indicator, add type hints (PR #89490)
https://github.com/nicovank created https://github.com/llvm/llvm-project/pull/89490 [There is work](https://discourse.llvm.org/t/rfc-upgrading-llvms-minimum-required-python-version/67571) to make Python 3.8 the minimum Python version for LLVM. I edited this script because I wanted some indicator of progress while going through files. It now outputs `[XX/YYY]` with the number of processed and total files after each completion. The current version of this script is compatible downto Python 3.6 (this is PyYAML's minimum version). It would probably work with older Python 3 versions with an older PyYAML or when YAML is disabled. With the updates here, it is compatible downto Python 3.7. Python 3.7 was released June 2018. https://github.com/llvm/llvm-project/pull/89302 is also touching this file, I don't mind rebasing on top of that work if needed. ### Summary - Add type annotations. - Replace `threading` + `queue` with `asyncio`. - **Add indicator of processed files over total files**. This is what I set out to do initially. - Only print the filename after completion, not the entire Clang-Tidy invocation command. I find this neater but the behavior can easily be restored. ### MyPy ``` % python3 -m mypy --strict clang-tools-extra/clang-tidy/tool/run-clang-tidy.py Success: no issues found in 1 source file ``` ### Performance Just to make sure the `asyncio` change didn't introduce a regression, I ran the previous version and this version 5 times with the following command. ``` time python3 ~/llvm-project/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py \ -clang-tidy-binary="~/llvm-project/build/bin/clang-tidy" \ -clang-apply-replacements-binary="~/llvm-project/build/bin/clang-apply-replacements" \ -checks="-*,modernize-use-starts-ends-with" \ -source-filter ".*clang-tools-extra/clang-tidy.*" \ -fix -format ``` Runtimes are identical, no performance regression detected on my setup. | Version | Time (s) | |-|| | Old | 92.0 ± 1.4 | | New | 92.2 ± 1.5 | >From 8decb15cb6a1734b01e31277d4698677564b227e Mon Sep 17 00:00:00 2001 From: Nicolas van Kempen Date: Sat, 20 Apr 2024 02:58:25 + Subject: [PATCH] [run-clang-tidy.py] Refactor, add progress indicator, add type hints --- .../clang-tidy/tool/run-clang-tidy.py | 248 +- 1 file changed, 126 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index 1bd4a5b283091c..530f2425322495 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -34,29 +34,30 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ -from __future__ import print_function - import argparse +import asyncio import glob import json import multiprocessing import os -import queue import re import shutil import subprocess import sys import tempfile -import threading import traceback +from types import ModuleType +from typing import Any, Awaitable, Callable, List, Optional, Tuple, TypeVar + +yaml: Optional[ModuleType] = None try: import yaml except ImportError: -yaml = None +pass -def strtobool(val): +def strtobool(val: str) -> bool: """Convert a string representation of truth to a bool following LLVM's CLI argument parsing.""" val = val.lower() @@ -67,11 +68,11 @@ def strtobool(val): # Return ArgumentTypeError so that argparse does not substitute its own error message raise argparse.ArgumentTypeError( -"'{}' is invalid value for boolean argument! Try 0 or 1.".format(val) +f"'{val}' is invalid value for boolean argument! Try 0 or 1." ) -def find_compilation_database(path): +def find_compilation_database(path: str) -> str: """Adjusts the directory until a compilation database is found.""" result = os.path.realpath("./") while not os.path.isfile(os.path.join(result, path)): @@ -83,30 +84,24 @@ def find_compilation_database(path): return result -def make_absolute(f, directory): -if os.path.isabs(f): -return f -return os.path.normpath(os.path.join(directory, f)) - - def get_tidy_invocation( -f, -clang_tidy_binary, -checks, -tmpdir, -build_path, -header_filter, -allow_enabling_alpha_checkers, -extra_arg, -extra_arg_before, -quiet, -config_file_path, -config, -line_filter, -use_color, -plugins, -warnings_as_errors, -): +f: str, +clang_tidy_binary: str, +checks: str, +tmpdir: Optional[str], +build_path: str, +header_filter: Optional[str], +allow_enabling_alpha_checkers: bool, +extra_arg: List[str], +extra_arg_before: List[str], +quiet: bool, +config_file_path: str, +config: str, +line_filter: Optional[str], +use_color: bool, +plugins: List[str], +warnings_as_errors: