From 038b18edf195ecda7eb30d8252efffa61c36db2f Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Thu, 1 Aug 2019 16:09:26 -0400 Subject: [PATCH] Adding style scripts. (#243) * Adding style scripts. --- .drone.yml | 21 +- .travis.yml | 4 +- benchmark/linux/linux-perf-events.h | 10 +- include/simdjson/parsedjson.h | 5 +- include/simdjson/simdutf8check_haswell.h | 40 +-- include/simdjson/simdutf8check_westmere.h | 20 +- include/simdjson/stage1_find_marks_haswell.h | 2 + src/jsonparser.cpp | 2 +- src/parsedjsoniterator.cpp | 2 +- style/clang-format-check.sh | 25 ++ style/clang-format.sh | 25 ++ style/run-clang-format.py | 325 +++++++++++++++++++ 12 files changed, 439 insertions(+), 42 deletions(-) create mode 100755 style/clang-format-check.sh create mode 100755 style/clang-format.sh create mode 100755 style/run-clang-format.py diff --git a/.drone.yml b/.drone.yml index bc2719566..198c1784c 100644 --- a/.drone.yml +++ b/.drone.yml @@ -29,6 +29,21 @@ steps: - make amalgamate --- kind: pipeline +name: stylecheck + +platform: + os: linux + arch: amd64 + +steps: +- name: Build and Test + image: ubuntu:18.04 + commands: + - apt-get update -y + - apt-get install -y python clang-format + - ./style/run-clang-format.py -r include/ benchmark/ src/ tests/ +--- +kind: pipeline name: amd64_clang_cmake platform: @@ -43,7 +58,7 @@ steps: CXX: clang++ commands: - apt-get update -y - - apt-get install -y make $CC g++ cmake + - apt-get install -y make $CC g++ cmake - $CC --version - mkdir build && cd build - cmake $CMAKE_FLAGS .. @@ -65,7 +80,7 @@ steps: CXX: g++ commands: - apt-get update -y - - apt-get install -y make $CC g++ cmake + - apt-get install -y make $CC g++ cmake - $CC --version - mkdir build && cd build - cmake $CMAKE_FLAGS .. @@ -87,7 +102,7 @@ steps: CXX: clang++ commands: - apt-get update -y - - apt-get install -y make $CC g++ cmake + - apt-get install -y make $CC g++ cmake - $CC --version - mkdir build && cd build - cmake $CMAKE_FLAGS .. diff --git a/.travis.yml b/.travis.yml index 8628d15e5..aa3a73bf8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,8 @@ addons: packages: - gcc-7 - g++-7 - + - clang-format + - python branches: only: - master @@ -25,4 +26,5 @@ script: - ARCHFLAGS="-march=nehalem" make - ARCHFLAGS="-march=nehalem" make test - ARCHFLAGS="-march=nehalem" make everything + - ./style/run-clang-format.py -r include/ benchmark/ src/ tests/ diff --git a/benchmark/linux/linux-perf-events.h b/benchmark/linux/linux-perf-events.h index bedd4a3a2..0abda3d41 100644 --- a/benchmark/linux/linux-perf-events.h +++ b/benchmark/linux/linux-perf-events.h @@ -11,16 +11,17 @@ #include // for memset #include -#include #include +#include template class LinuxEvents { int fd; - bool working; + bool working; perf_event_attr attribs; int num_events; std::vector temp_result_vec; std::vector ids; + public: explicit LinuxEvents(std::vector config_vec) : fd(0), working(true) { memset(&attribs, 0, sizeof(attribs)); @@ -84,8 +85,9 @@ public: private: void report_error(const std::string &context) { - if(working) std::cerr << (context + ": " + std::string(strerror(errno))) << std::endl; - working = false; + if (working) + std::cerr << (context + ": " + std::string(strerror(errno))) << std::endl; + working = false; } }; #endif diff --git a/include/simdjson/parsedjson.h b/include/simdjson/parsedjson.h index 81f06d50c..9e7bd73ca 100644 --- a/include/simdjson/parsedjson.h +++ b/include/simdjson/parsedjson.h @@ -7,10 +7,10 @@ #include "simdjson/simdjson.h" #include #include -#include #include #include #include +#include #define JSON_VALUE_MASK 0xFFFFFFFFFFFFFF @@ -168,7 +168,8 @@ public: // we're at "d" inline double get_double() const { if (location + 1 >= tape_length) { - return std::numeric_limits::quiet_NaN(); // default value in case of error + return std::numeric_limits::quiet_NaN(); // default value in + // case of error } double answer; memcpy(&answer, &pj.tape[location + 1], sizeof(answer)); diff --git a/include/simdjson/simdutf8check_haswell.h b/include/simdjson/simdutf8check_haswell.h index 920730993..6097af0c4 100644 --- a/include/simdjson/simdutf8check_haswell.h +++ b/include/simdjson/simdutf8check_haswell.h @@ -119,31 +119,31 @@ static inline void avx_check_overlong(__m256i current_bytes, __m256i *has_error) { __m256i off1_hibits = push_last_byte_of_a_to_b(previous_hibits, hibits); __m256i initial_mins = _mm256_shuffle_epi8( - _mm256_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, -128, // 10xx => false - 0xC2u, -128, // 110x - 0xE1u, // 1110 - 0xF1u, // 1111 - -128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, -128, // 10xx => false - 0xC2u, -128, // 110x - 0xE1u, // 1110 - 0xF1u), // 1111 + _mm256_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, -128, // 10xx => false + 0xC2u, -128, // 110x + 0xE1u, // 1110 + 0xF1u, // 1111 + -128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, -128, // 10xx => false + 0xC2u, -128, // 110x + 0xE1u, // 1110 + 0xF1u), // 1111 off1_hibits); __m256i initial_under = _mm256_cmpgt_epi8(initial_mins, off1_current_bytes); __m256i second_mins = _mm256_shuffle_epi8( - _mm256_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, -128, // 10xx => false - 127, 127, // 110x => true - 0xA0u, // 1110 - 0x90u, // 1111 - -128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, -128, // 10xx => false - 127, 127, // 110x => true - 0xA0u, // 1110 - 0x90u), // 1111 + _mm256_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, -128, // 10xx => false + 127, 127, // 110x => true + 0xA0u, // 1110 + 0x90u, // 1111 + -128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, -128, // 10xx => false + 127, 127, // 110x => true + 0xA0u, // 1110 + 0x90u), // 1111 off1_hibits); __m256i second_under = _mm256_cmpgt_epi8(second_mins, current_bytes); *has_error = _mm256_or_si256(*has_error, diff --git a/include/simdjson/simdutf8check_westmere.h b/include/simdjson/simdutf8check_westmere.h index 20d5b8836..cf57fec9a 100644 --- a/include/simdjson/simdutf8check_westmere.h +++ b/include/simdjson/simdutf8check_westmere.h @@ -83,10 +83,10 @@ static inline void check_first_continuation_max(__m128i current_bytes, __m128i maskED = _mm_cmpeq_epi8(off1_current_bytes, _mm_set1_epi8(0xEDu)); __m128i maskF4 = _mm_cmpeq_epi8(off1_current_bytes, _mm_set1_epi8(0xF4u)); - __m128i badfollowED = - _mm_and_si128(_mm_cmpgt_epi8(current_bytes, _mm_set1_epi8(0x9Fu)), maskED); - __m128i badfollowF4 = - _mm_and_si128(_mm_cmpgt_epi8(current_bytes, _mm_set1_epi8(0x8Fu)), maskF4); + __m128i badfollowED = _mm_and_si128( + _mm_cmpgt_epi8(current_bytes, _mm_set1_epi8(0x9Fu)), maskED); + __m128i badfollowF4 = _mm_and_si128( + _mm_cmpgt_epi8(current_bytes, _mm_set1_epi8(0x8Fu)), maskF4); *has_error = _mm_or_si128(*has_error, _mm_or_si128(badfollowED, badfollowF4)); } @@ -102,8 +102,8 @@ static inline void check_overlong(__m128i current_bytes, __m128i previous_hibits, __m128i *has_error) { __m128i off1_hibits = _mm_alignr_epi8(hibits, previous_hibits, 16 - 1); __m128i initial_mins = _mm_shuffle_epi8( - _mm_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, // 10xx => false + _mm_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, // 10xx => false 0xC2u, -128, // 110x 0xE1u, // 1110 0xF1u), @@ -112,10 +112,10 @@ static inline void check_overlong(__m128i current_bytes, __m128i initial_under = _mm_cmpgt_epi8(initial_mins, off1_current_bytes); __m128i second_mins = _mm_shuffle_epi8( - _mm_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, -128, - -128, -128, // 10xx => false - 127, 127, // 110x => true - 0xA0u, // 1110 + _mm_setr_epi8(-128, -128, -128, -128, -128, -128, -128, -128, -128, -128, + -128, -128, // 10xx => false + 127, 127, // 110x => true + 0xA0u, // 1110 0x90u), off1_hibits); __m128i second_under = _mm_cmpgt_epi8(second_mins, current_bytes); diff --git a/include/simdjson/stage1_find_marks_haswell.h b/include/simdjson/stage1_find_marks_haswell.h index 64eeecafc..3be1ddc0d 100644 --- a/include/simdjson/stage1_find_marks_haswell.h +++ b/include/simdjson/stage1_find_marks_haswell.h @@ -169,12 +169,14 @@ really_inline void find_whitespace_and_structurals( // end of naive approach #else // SIMDJSON_NAIVE_STRUCTURAL + // clang-format off const __m256i structural_table = _mm256_setr_epi8(44, 125, 0, 0, 0xc0u, 0, 0, 0, 0, 0, 0, 0, 0, 0, 58, 123, 44, 125, 0, 0, 0xc0u, 0, 0, 0, 0, 0, 0, 0, 0, 0, 58, 123); const __m256i white_table = _mm256_setr_epi8( 32, 100, 100, 100, 17, 100, 113, 2, 100, 9, 10, 112, 100, 13, 100, 100, 32, 100, 100, 100, 17, 100, 113, 2, 100, 9, 10, 112, 100, 13, 100, 100); + // clang-format on const __m256i struct_offset = _mm256_set1_epi8(0xd4u); const __m256i struct_mask = _mm256_set1_epi8(32); diff --git a/src/jsonparser.cpp b/src/jsonparser.cpp index 4e1bd0716..9e263749e 100644 --- a/src/jsonparser.cpp +++ b/src/jsonparser.cpp @@ -1,5 +1,5 @@ -#include "simdjson/isadetection.h" #include "simdjson/jsonparser.h" +#include "simdjson/isadetection.h" #include "simdjson/portability.h" #include "simdjson/simdjson.h" diff --git a/src/parsedjsoniterator.cpp b/src/parsedjsoniterator.cpp index 35c9654d0..e8f0666c9 100644 --- a/src/parsedjsoniterator.cpp +++ b/src/parsedjsoniterator.cpp @@ -20,7 +20,7 @@ ParsedJson::Iterator::Iterator(ParsedJson &pj_) depth_index[0].scope_type = current_type; if (current_type == 'r') { tape_length = current_val & JSON_VALUE_MASK; - if (location < tape_length) { + if (location < tape_length) { // If we make it here, then depth_capacity must >=2, but the compiler // may not know this. current_val = pj.tape[location]; diff --git a/style/clang-format-check.sh b/style/clang-format-check.sh new file mode 100755 index 000000000..b9445f7fd --- /dev/null +++ b/style/clang-format-check.sh @@ -0,0 +1,25 @@ +#!/bin/bash +SCRIPTPATH="$( cd "$(dirname "$0")" ; pwd -P )" +BASE=$SCRIPTPATH/.. +cd $BASE + +STYLE=$(which clang-format) +if [ $? -ne 0 ]; then + echo "clang-format not installed. Unable to check source file format policy." >&2 + exit 1 +fi +OURSTYLE='' # defer to .clang-format +OURCONTENT="include benchmark tools tests src" +RE=0 +ALLFILES=$(find $OURCONTENT -name '*.c' -o -name '*.cpp' -o -name '*.h' -o -name '*.cc' -o -name '*.hh') +for FILE in $ALLFILES; do + echo "checking $FILE" + eval "$STYLE $OURSTYLE $BASE/$FILE" | cmp -s $BASE/$FILE - + if [ $? -ne 0 ]; then + echo "$BASE/$FILE does not respect the coding style." >&2 + echo "consider typing $STYLE -i $BASE/$FILE $OURSTYLE to fix the problem." >&2 + RE=1 + fi +done + +exit $RE diff --git a/style/clang-format.sh b/style/clang-format.sh new file mode 100755 index 000000000..4125f8770 --- /dev/null +++ b/style/clang-format.sh @@ -0,0 +1,25 @@ +#!/bin/bash +SCRIPTPATH="$( cd "$(dirname "$0")" ; pwd -P )" +BASE=$SCRIPTPATH/.. +cd $BASE + +STYLE=$(which clang-format) +if [ $? -ne 0 ]; then + echo "clang-format not installed. Unable to check source file format policy." >&2 + exit 1 +fi +OURSTYLE="" # defer to .clang-format +OURCONTENT="include benchmark tools tests src" +RE=0 +BASE=$(git rev-parse --show-toplevel) +ALLFILES=$(find $OURCONTENT -name '*.c' -o -name '*.cpp' -o -name '*.h' -o -name '*.cc' -o -name '*.hh') +for FILE in $ALLFILES; do + eval "$STYLE $OURSTYLE $BASE/$FILE" | cmp -s $BASE/$FILE - + if [ $? -ne 0 ]; then + echo "$BASE/$FILE does not respect the coding style. Formatting. " >&2 + eval "$STYLE $OURSTYLE -i $BASE/$FILE" + RE=1 + fi +done + +exit $RE diff --git a/style/run-clang-format.py b/style/run-clang-format.py new file mode 100755 index 000000000..3d0cca0a9 --- /dev/null +++ b/style/run-clang-format.py @@ -0,0 +1,325 @@ +#!/usr/bin/env python +"""A wrapper script around clang-format, suitable for linting multiple files +and to use for continuous integration. + +This is an alternative API for the clang-format command line. +It runs over multiple files and directories in parallel. +A diff output is produced and a sensible exit code is returned. + +""" + +from __future__ import print_function, unicode_literals + +import argparse +import codecs +import difflib +import fnmatch +import io +import multiprocessing +import os +import signal +import subprocess +import sys +import traceback + +from functools import partial + +DEFAULT_EXTENSIONS = 'c,h,C,H,cpp,hpp,cc,hh,c++,h++,cxx,hxx' + + +class ExitStatus: + SUCCESS = 0 + DIFF = 1 + TROUBLE = 2 + + +def list_files(files, recursive=False, extensions=None, exclude=None): + if extensions is None: + extensions = [] + if exclude is None: + exclude = [] + + out = [] + for file in files: + if recursive and os.path.isdir(file): + for dirpath, dnames, fnames in os.walk(file): + fpaths = [os.path.join(dirpath, fname) for fname in fnames] + for pattern in exclude: + # os.walk() supports trimming down the dnames list + # by modifying it in-place, + # to avoid unnecessary directory listings. + dnames[:] = [ + x for x in dnames + if + not fnmatch.fnmatch(os.path.join(dirpath, x), pattern) + ] + fpaths = [ + x for x in fpaths if not fnmatch.fnmatch(x, pattern) + ] + for f in fpaths: + ext = os.path.splitext(f)[1][1:] + if ext in extensions: + out.append(f) + else: + out.append(file) + return out + + +def make_diff(file, original, reformatted): + return list( + difflib.unified_diff( + original, + reformatted, + fromfile='{}\t(original)'.format(file), + tofile='{}\t(reformatted)'.format(file), + n=3)) + + +class DiffError(Exception): + def __init__(self, message, errs=None): + super(DiffError, self).__init__(message) + self.errs = errs or [] + + +class UnexpectedError(Exception): + def __init__(self, message, exc=None): + super(UnexpectedError, self).__init__(message) + self.formatted_traceback = traceback.format_exc() + self.exc = exc + + +def run_clang_format_diff_wrapper(args, file): + try: + ret = run_clang_format_diff(args, file) + return ret + except DiffError: + raise + except Exception as e: + raise UnexpectedError('{}: {}: {}'.format(file, e.__class__.__name__, + e), e) + + +def run_clang_format_diff(args, file): + try: + with io.open(file, 'r', encoding='utf-8') as f: + original = f.readlines() + except IOError as exc: + raise DiffError(str(exc)) + invocation = [args.clang_format_executable, file] + + # Use of utf-8 to decode the process output. + # + # Hopefully, this is the correct thing to do. + # + # It's done due to the following assumptions (which may be incorrect): + # - clang-format will returns the bytes read from the files as-is, + # without conversion, and it is already assumed that the files use utf-8. + # - if the diagnostics were internationalized, they would use utf-8: + # > Adding Translations to Clang + # > + # > Not possible yet! + # > Diagnostic strings should be written in UTF-8, + # > the client can translate to the relevant code page if needed. + # > Each translation completely replaces the format string + # > for the diagnostic. + # > -- http://clang.llvm.org/docs/InternalsManual.html#internals-diag-translation + # + # It's not pretty, due to Python 2 & 3 compatibility. + encoding_py3 = {} + if sys.version_info[0] >= 3: + encoding_py3['encoding'] = 'utf-8' + + try: + proc = subprocess.Popen( + invocation, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + **encoding_py3) + except OSError as exc: + raise DiffError(str(exc)) + proc_stdout = proc.stdout + proc_stderr = proc.stderr + if sys.version_info[0] < 3: + # make the pipes compatible with Python 3, + # reading lines should output unicode + encoding = 'utf-8' + proc_stdout = codecs.getreader(encoding)(proc_stdout) + proc_stderr = codecs.getreader(encoding)(proc_stderr) + # hopefully the stderr pipe won't get full and block the process + outs = list(proc_stdout.readlines()) + errs = list(proc_stderr.readlines()) + proc.wait() + if proc.returncode: + raise DiffError("clang-format exited with status {}: '{}'".format( + proc.returncode, file), errs) + return make_diff(file, original, outs), errs + + +def bold_red(s): + return '\x1b[1m\x1b[31m' + s + '\x1b[0m' + + +def colorize(diff_lines): + def bold(s): + return '\x1b[1m' + s + '\x1b[0m' + + def cyan(s): + return '\x1b[36m' + s + '\x1b[0m' + + def green(s): + return '\x1b[32m' + s + '\x1b[0m' + + def red(s): + return '\x1b[31m' + s + '\x1b[0m' + + for line in diff_lines: + if line[:4] in ['--- ', '+++ ']: + yield bold(line) + elif line.startswith('@@ '): + yield cyan(line) + elif line.startswith('+'): + yield green(line) + elif line.startswith('-'): + yield red(line) + else: + yield line + + +def print_diff(diff_lines, use_color): + if use_color: + diff_lines = colorize(diff_lines) + if sys.version_info[0] < 3: + sys.stdout.writelines((l.encode('utf-8') for l in diff_lines)) + else: + sys.stdout.writelines(diff_lines) + + +def print_trouble(prog, message, use_colors): + error_text = 'error:' + if use_colors: + error_text = bold_red(error_text) + print("{}: {} {}".format(prog, error_text, message), file=sys.stderr) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + '--clang-format-executable', + metavar='EXECUTABLE', + help='path to the clang-format executable', + default='clang-format') + parser.add_argument( + '--extensions', + help='comma separated list of file extensions (default: {})'.format( + DEFAULT_EXTENSIONS), + default=DEFAULT_EXTENSIONS) + parser.add_argument( + '-r', + '--recursive', + action='store_true', + help='run recursively over directories') + parser.add_argument('files', metavar='file', nargs='+') + parser.add_argument( + '-q', + '--quiet', + action='store_true') + parser.add_argument( + '-j', + metavar='N', + type=int, + default=0, + help='run N clang-format jobs in parallel' + ' (default number of cpus + 1)') + parser.add_argument( + '--color', + default='auto', + choices=['auto', 'always', 'never'], + help='show colored diff (default: auto)') + parser.add_argument( + '-e', + '--exclude', + metavar='PATTERN', + action='append', + default=[], + help='exclude paths matching the given glob-like pattern(s)' + ' from recursive search') + + args = parser.parse_args() + + # use default signal handling, like diff return SIGINT value on ^C + # https://bugs.python.org/issue14229#msg156446 + signal.signal(signal.SIGINT, signal.SIG_DFL) + try: + signal.SIGPIPE + except AttributeError: + # compatibility, SIGPIPE does not exist on Windows + pass + else: + signal.signal(signal.SIGPIPE, signal.SIG_DFL) + + colored_stdout = False + colored_stderr = False + if args.color == 'always': + colored_stdout = True + colored_stderr = True + elif args.color == 'auto': + colored_stdout = sys.stdout.isatty() + colored_stderr = sys.stderr.isatty() + + retcode = ExitStatus.SUCCESS + files = list_files( + args.files, + recursive=args.recursive, + exclude=args.exclude, + extensions=args.extensions.split(',')) + + if not files: + return + + njobs = args.j + if njobs == 0: + njobs = multiprocessing.cpu_count() + 1 + njobs = min(len(files), njobs) + + if njobs == 1: + # execute directly instead of in a pool, + # less overhead, simpler stacktraces + it = (run_clang_format_diff_wrapper(args, file) for file in files) + pool = None + else: + pool = multiprocessing.Pool(njobs) + it = pool.imap_unordered( + partial(run_clang_format_diff_wrapper, args), files) + while True: + try: + outs, errs = next(it) + except StopIteration: + break + except DiffError as e: + print_trouble(parser.prog, str(e), use_colors=colored_stderr) + retcode = ExitStatus.TROUBLE + sys.stderr.writelines(e.errs) + except UnexpectedError as e: + print_trouble(parser.prog, str(e), use_colors=colored_stderr) + sys.stderr.write(e.formatted_traceback) + retcode = ExitStatus.TROUBLE + # stop at the first unexpected error, + # something could be very wrong, + # don't process all files unnecessarily + if pool: + pool.terminate() + break + else: + sys.stderr.writelines(errs) + if outs == []: + continue + if not args.quiet: + print_diff(outs, use_color=colored_stdout) + if retcode == ExitStatus.SUCCESS: + retcode = ExitStatus.DIFF + return retcode + + +if __name__ == '__main__': + sys.exit(main())