From 8546d6089a89a4e34219fc10af088394a2eb3b1c Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 11 Mar 2020 13:33:54 +0100 Subject: [PATCH] Update clang-tidy configuration and scripts --- .clang-tidy | 7 +- util/travis/clangtidy.sh | 12 +- util/travis/run-clang-tidy.py | 452 +++++++++++++++++++--------------- 3 files changed, 260 insertions(+), 211 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 80c1949f9..1b9f8bd07 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,5 @@ -Checks: '-*,modernize-use-emplace,modernize-use-default-member-init,modernize-use-equals-delete,modernize-use-equals-default,modernize-return-braced-init-list,modernize-loop-convert,modernize-avoid-bind,misc-throw-by-value-catch-by-reference,misc-string-compare,misc-inefficient-algorithm,misc-inaccurate-erase,misc-incorrect-roundings,misc-unconventional-assign-operator,bugprone-suspicious-memset-usage,performance-*' +Checks: '-*,modernize-use-emplace,modernize-avoid-bind,misc-throw-by-value-catch-by-reference,misc-unconventional-assign-operator,performance-*' +WarningsAsErrors: '-*,modernize-use-emplace,performance-type-promotion-in-math-fn,performance-faster-string-find,performance-implicit-cast-in-loop' CheckOptions: - - key: modernize-use-default-member-init.UseAssignment - value: True + - key: performance-unnecessary-value-param.AllowedTypes + value: v[23]f;v[23][su](16|32) diff --git a/util/travis/clangtidy.sh b/util/travis/clangtidy.sh index 5b00115c8..ed6523b0b 100755 --- a/util/travis/clangtidy.sh +++ b/util/travis/clangtidy.sh @@ -7,8 +7,6 @@ if [ -z "${CLANG_TIDY}" ]; then CLANG_TIDY=clang-tidy fi -files_to_analyze="$(find src/ -name '*.cpp' -or -name '*.h')" - mkdir -p cmakebuild && cd cmakebuild cmake -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ @@ -20,11 +18,11 @@ make GenerateVersion cd .. echo "Performing clang-tidy checks..." -./util/travis/run-clang-tidy.py -clang-tidy-binary=${CLANG_TIDY} -p cmakebuild \ - -checks='-*,modernize-use-emplace,modernize-avoid-bind,performance-*' \ - -warningsaserrors='-*,modernize-use-emplace,performance-type-promotion-in-math-fn,performance-faster-string-find,performance-implicit-cast-in-loop' \ - -no-command-on-stdout -quiet \ - files 'src/.*' +./util/travis/run-clang-tidy.py \ + -clang-tidy-binary=${CLANG_TIDY} -p cmakebuild \ + -quiet -config="$(cat .clang-tidy)" \ + 'src/.*' + RET=$? echo "Clang tidy returned $RET" exit $RET diff --git a/util/travis/run-clang-tidy.py b/util/travis/run-clang-tidy.py index 779cf7cc6..6ad0ff24f 100755 --- a/util/travis/run-clang-tidy.py +++ b/util/travis/run-clang-tidy.py @@ -1,13 +1,12 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python # -# ===- run-clang-tidy.py - Parallel clang-tidy runner ---------*- python -*--===# +#===- run-clang-tidy.py - Parallel clang-tidy runner ---------*- python -*--===# # -# The LLVM Compiler Infrastructure +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception # -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# -# ===------------------------------------------------------------------------===# +#===------------------------------------------------------------------------===# # FIXME: Integrate with clang-tidy-diff.py """ @@ -35,11 +34,12 @@ http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html """ from __future__ import print_function + import argparse +import glob import json import multiprocessing import os -import Queue import re import shutil import subprocess @@ -48,224 +48,274 @@ import tempfile import threading import traceback +try: + import yaml +except ImportError: + yaml = None -class TidyQueue(Queue.Queue): - def __init__(self, max_task): - Queue.Queue.__init__(self, max_task) - self.has_error = False +is_py2 = sys.version[0] == '2' +if is_py2: + import Queue as queue +else: + import queue as queue def find_compilation_database(path): - """Adjusts the directory until a compilation database is found.""" - result = './' - while not os.path.isfile(os.path.join(result, path)): - if os.path.realpath(result) == '/': - print('Error: could not find compilation database.') - sys.exit(1) - result += '../' - return os.path.realpath(result) + """Adjusts the directory until a compilation database is found.""" + result = './' + while not os.path.isfile(os.path.join(result, path)): + if os.path.realpath(result) == '/': + print('Error: could not find compilation database.') + sys.exit(1) + result += '../' + return os.path.realpath(result) -def get_tidy_invocation(f, clang_tidy_binary, checks, warningsaserrors, - tmpdir, build_path, - header_filter, extra_arg, extra_arg_before, quiet): - """Gets a command line for clang-tidy.""" - start = [clang_tidy_binary] - if header_filter is not None: - start.append('-header-filter=' + header_filter) - else: - # Show warnings in all in-project headers by default. - start.append('-header-filter=^' + build_path + '/.*') - if checks: - start.append('-checks=' + checks) - if warningsaserrors: - start.append('-warnings-as-errors=' + warningsaserrors) - if tmpdir is not None: - start.append('-export-fixes') - # Get a temporary file. We immediately close the handle so clang-tidy can - # overwrite it. - (handle, name) = tempfile.mkstemp(suffix='.yaml', dir=tmpdir) - os.close(handle) - start.append(name) - for arg in extra_arg: - start.append('-extra-arg=%s' % arg) - for arg in extra_arg_before: - start.append('-extra-arg-before=%s' % arg) - start.append('-p=' + build_path) - if quiet: - start.append('-quiet') - start.append(f) - return start +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, extra_arg, extra_arg_before, quiet, + config): + """Gets a command line for clang-tidy.""" + start = [clang_tidy_binary] + if header_filter is not None: + start.append('-header-filter=' + header_filter) + if checks: + start.append('-checks=' + checks) + if tmpdir is not None: + start.append('-export-fixes') + # Get a temporary file. We immediately close the handle so clang-tidy can + # overwrite it. + (handle, name) = tempfile.mkstemp(suffix='.yaml', dir=tmpdir) + os.close(handle) + start.append(name) + for arg in extra_arg: + start.append('-extra-arg=%s' % arg) + for arg in extra_arg_before: + start.append('-extra-arg-before=%s' % arg) + start.append('-p=' + build_path) + if quiet: + start.append('-quiet') + if config: + start.append('-config=' + config) + start.append(f) + return 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 + mergekey="Diagnostics" + merged=[] + for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')): + content = yaml.safe_load(open(replacefile, 'r')) + if not content: + continue # Skip empty files. + merged.extend(content.get(mergekey, [])) + + if merged: + # MainSourceFile: The key is required by the definition inside + # include/clang/Tooling/ReplacementsYaml.h, but the value + # is actually never used inside clang-apply-replacements, + # so we set it to '' here. + output = { 'MainSourceFile': '', mergekey: merged } + with open(mergefile, 'w') as out: + yaml.safe_dump(output, out) + else: + # Empty the file: + open(mergefile, 'w').close() def check_clang_apply_replacements_binary(args): - """Checks if invoking supplied clang-apply-replacements binary works.""" - try: - subprocess.check_call([args.clang_apply_replacements_binary, '--version']) - except: - print('Unable to run clang-apply-replacements. Is clang-apply-replacements ' - 'binary correctly specified?', file=sys.stderr) - traceback.print_exc() - sys.exit(1) + """Checks if invoking supplied clang-apply-replacements binary works.""" + try: + subprocess.check_call([args.clang_apply_replacements_binary, '--version']) + except: + print('Unable to run clang-apply-replacements. Is clang-apply-replacements ' + 'binary correctly specified?', file=sys.stderr) + traceback.print_exc() + sys.exit(1) def apply_fixes(args, tmpdir): - """Calls clang-apply-fixes on a given directory. Deletes the dir when done.""" - invocation = [args.clang_apply_replacements_binary] - if args.format: - invocation.append('-format') - if args.style: - invocation.append('-style=' + args.style) - invocation.append(tmpdir) - subprocess.call(invocation) + """Calls clang-apply-fixes on a given directory.""" + invocation = [args.clang_apply_replacements_binary] + if args.format: + invocation.append('-format') + if args.style: + invocation.append('-style=' + args.style) + invocation.append(tmpdir) + subprocess.call(invocation) -def run_tidy(args, tmpdir, build_path, queue): - """Takes filenames out of queue and runs clang-tidy on them.""" - while True: - name = queue.get() - invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks, - args.warningsaserrors, tmpdir, build_path, - args.header_filter, args.extra_arg, - args.extra_arg_before, args.quiet) - if not args.no_command_on_stdout: - sys.stdout.write(' '.join(invocation) + '\n') - try: - subprocess.check_call(invocation) - except subprocess.CalledProcessError: - queue.has_error = True - queue.task_done() +def run_tidy(args, tmpdir, build_path, queue, lock, failed_files): + """Takes filenames out of queue and runs clang-tidy on them.""" + while True: + name = queue.get() + invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks, + tmpdir, build_path, args.header_filter, + args.extra_arg, args.extra_arg_before, + args.quiet, args.config) + + proc = subprocess.Popen(invocation) + proc.wait() + if proc.returncode != 0: + failed_files.append(name) + queue.task_done() def main(): - parser = argparse.ArgumentParser(description='Runs clang-tidy over all files ' - 'in a compilation database. Requires ' - 'clang-tidy and clang-apply-replacements in ' - '$PATH.') - parser.add_argument('-clang-tidy-binary', metavar='PATH', - default='clang-tidy', - help='path to clang-tidy binary') - parser.add_argument('-clang-apply-replacements-binary', metavar='PATH', - default='clang-apply-replacements', - help='path to clang-apply-replacements binary') - parser.add_argument('-checks', default=None, - help='checks filter, when not specified, use clang-tidy ' - 'default') - parser.add_argument('-warningsaserrors', default=None, - help='warnings-as-errors filter, when not specified, ' - 'use clang-tidy default') - parser.add_argument('-header-filter', default=None, - help='regular expression matching the names of the ' - 'headers to output diagnostics from. Diagnostics from ' - 'the main file of each translation unit are always ' - 'displayed.') - parser.add_argument('-j', type=int, default=0, - help='number of tidy instances to be run in parallel.') - parser.add_argument('files', nargs='*', default=['.*'], - help='files to be processed (regex on path)') - parser.add_argument('-fix', action='store_true', help='apply fix-its') - parser.add_argument('-format', action='store_true', help='Reformat code ' - 'after applying fixes') - parser.add_argument('-style', default='file', help='The style of reformat ' - 'code after applying fixes') - parser.add_argument('-p', dest='build_path', - help='Path used to read a compile command database.') - parser.add_argument('-extra-arg', dest='extra_arg', - action='append', default=[], - help='Additional argument to append to the compiler ' - 'command line.') - parser.add_argument('-extra-arg-before', dest='extra_arg_before', - action='append', default=[], - help='Additional argument to prepend to the compiler ' - 'command line.') - parser.add_argument('-quiet', action='store_true', - help='Run clang-tidy in quiet mode') - parser.add_argument('-no-command-on-stdout', action='store_true', - help='Run clang-tidy without printing invocation on ' - 'stdout') - args = parser.parse_args() + parser = argparse.ArgumentParser(description='Runs clang-tidy over all files ' + 'in a compilation database. Requires ' + 'clang-tidy and clang-apply-replacements in ' + '$PATH.') + parser.add_argument('-clang-tidy-binary', metavar='PATH', + default='clang-tidy', + help='path to clang-tidy binary') + parser.add_argument('-clang-apply-replacements-binary', metavar='PATH', + default='clang-apply-replacements', + help='path to clang-apply-replacements binary') + parser.add_argument('-checks', default=None, + help='checks filter, when not specified, use clang-tidy ' + 'default') + parser.add_argument('-config', default=None, + help='Specifies a configuration in YAML/JSON format: ' + ' -config="{Checks: \'*\', ' + ' CheckOptions: [{key: x, ' + ' value: y}]}" ' + 'When the value is empty, clang-tidy will ' + 'attempt to find a file named .clang-tidy for ' + 'each source file in its parent directories.') + parser.add_argument('-header-filter', default=None, + help='regular expression matching the names of the ' + 'headers to output diagnostics from. Diagnostics from ' + 'the main file of each translation unit are always ' + 'displayed.') + if yaml: + parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes', + help='Create a yaml file to store suggested fixes in, ' + 'which can be applied with clang-apply-replacements.') + parser.add_argument('-j', type=int, default=0, + help='number of tidy instances to be run in parallel.') + parser.add_argument('files', nargs='*', default=['.*'], + help='files to be processed (regex on path)') + parser.add_argument('-fix', action='store_true', help='apply fix-its') + parser.add_argument('-format', action='store_true', help='Reformat code ' + 'after applying fixes') + parser.add_argument('-style', default='file', help='The style of reformat ' + 'code after applying fixes') + parser.add_argument('-p', dest='build_path', + help='Path used to read a compile command database.') + parser.add_argument('-extra-arg', dest='extra_arg', + action='append', default=[], + help='Additional argument to append to the compiler ' + 'command line.') + parser.add_argument('-extra-arg-before', dest='extra_arg_before', + action='append', default=[], + help='Additional argument to prepend to the compiler ' + 'command line.') + parser.add_argument('-quiet', action='store_true', + help='Run clang-tidy in quiet mode') + args = parser.parse_args() - db_path = 'compile_commands.json' + db_path = 'compile_commands.json' - if args.build_path is not None: - build_path = args.build_path + if args.build_path is not None: + build_path = args.build_path + else: + # Find our database + build_path = find_compilation_database(db_path) + + try: + invocation = [args.clang_tidy_binary, '-list-checks'] + invocation.append('-p=' + build_path) + if args.checks: + invocation.append('-checks=' + args.checks) + invocation.append('-') + if args.quiet: + # Even with -quiet we still want to check if we can call clang-tidy. + with open(os.devnull, 'w') as dev_null: + subprocess.check_call(invocation, stdout=dev_null) else: - # Find our database - build_path = find_compilation_database(db_path) + subprocess.check_call(invocation) + except: + print("Unable to run clang-tidy.", file=sys.stderr) + sys.exit(1) + # Load the database and extract all files. + database = json.load(open(os.path.join(build_path, db_path))) + files = [make_absolute(entry['file'], entry['directory']) + for entry in database] + + max_task = args.j + if max_task == 0: + max_task = multiprocessing.cpu_count() + + tmpdir = None + if args.fix or (yaml and args.export_fixes): + check_clang_apply_replacements_binary(args) + tmpdir = tempfile.mkdtemp() + + # Build up a big regexy filter from all command line arguments. + file_name_re = re.compile('|'.join(args.files)) + + return_code = 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, tmpdir, build_path, task_queue, lock, failed_files)) + 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 + + 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 tmpdir: + shutil.rmtree(tmpdir) + os.kill(0, 9) + + if yaml and args.export_fixes: + print('Writing fixes to ' + args.export_fixes + ' ...') try: - invocation = [args.clang_tidy_binary, '-list-checks', '-p=' + build_path] - if args.checks: - invocation.append('-checks=' + args.checks) - if args.warningsaserrors: - invocation.append('-warnings-as-errors=' + args.warningsaserrors) - invocation.append('-') - print(subprocess.check_output(invocation)) + merge_replacement_files(tmpdir, args.export_fixes) except: - print("Unable to run clang-tidy.", file=sys.stderr) - sys.exit(1) - - # Load the database and extract all files. - database = json.load(open(os.path.join(build_path, db_path))) - files = [entry['file'] for entry in database] - - max_task = args.j - if max_task == 0: - max_task = multiprocessing.cpu_count() - - tmpdir = None - if args.fix: - check_clang_apply_replacements_binary(args) - tmpdir = tempfile.mkdtemp() - - # Build up a big regexy filter from all command line arguments. - file_name_re = re.compile('|'.join(args.files)) + print('Error exporting fixes.\n', file=sys.stderr) + traceback.print_exc() + return_code=1 + if args.fix: + print('Applying fixes ...') try: - # Spin up a bunch of tidy-launching threads. - queue = TidyQueue(max_task) - for _ in range(max_task): - t = threading.Thread(target=run_tidy, - args=(args, tmpdir, build_path, queue)) - t.daemon = True - t.start() - - # Fill the queue with files. - for name in files: - if file_name_re.search(name): - queue.put(name) - - # Wait for all threads to be done. - queue.join() - - # If one clang-tidy process found and error, exit with non-zero - # status - if queue.has_error: - sys.exit(2) - - 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 args.fix: - shutil.rmtree(tmpdir) - os.kill(0, 9) - - if args.fix: - print('Applying fixes ...') - successfully_applied = False - - try: - apply_fixes(args, tmpdir) - successfully_applied = True - except: - print('Error applying fixes.\n', file=sys.stderr) - traceback.print_exc() - - shutil.rmtree(tmpdir) - if not successfully_applied: - sys.exit(1) + apply_fixes(args, tmpdir) + except: + print('Error applying fixes.\n', file=sys.stderr) + traceback.print_exc() + return_code=1 + if tmpdir: + shutil.rmtree(tmpdir) + sys.exit(return_code) if __name__ == '__main__': - main() + main()