From d18759b9d0b62a6440e4d95622c4231dff964b32 Mon Sep 17 00:00:00 2001 From: John Freeman Date: Fri, 27 May 2022 11:38:56 -0500 Subject: [PATCH 1/4] JCF: add logic s.t. when clang-tidy recursively goes into an \#included header file, the warnings and errors for that header file only get printed out once, rather than for each place in the codebase that the include takes place --- cpplint/dune-cpp-style-check.sh | 24 +++++++++++++++--------- cpplint/duneclang-tidy.sh | 4 ++-- cpplint/duneclang-tidy_scrub_output.awk | 22 +++++++++++++++++----- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/cpplint/dune-cpp-style-check.sh b/cpplint/dune-cpp-style-check.sh index 58804721a..0170e557c 100755 --- a/cpplint/dune-cpp-style-check.sh +++ b/cpplint/dune-cpp-style-check.sh @@ -98,26 +98,32 @@ else ups_get_clang fi - files=$( echo $files | tr " " "\n" | sort | tr "\n" " " ) DIR="$(dirname "$(readlink -f "$0")")" for file in $files ; do - if [[ $file =~ .*/Structs.hpp || $file =~ .*/Nljs.hpp ]]; then - continue - fi + if [[ $file =~ .*/Structs.hpp || $file =~ .*/Nljs.hpp ]]; then + continue + fi + + echo + echo "Applying dunecpplint.sh" + $DIR/dunecpplint.sh $file + +done + +for file in $files ; do - echo - echo "Applying dunecpplint.sh" - $DIR/dunecpplint.sh $file if [[ "$file" =~ .*cxx$ || "$file" =~ .*cpp$ ]]; then echo - echo "Applying duneclang-tidy.sh" + # JCF, May-27-2022: use of carets in echo below because duneclang-tidy_scrub_output.awk breaks records with carets rather than newlines + echo "^Applying duneclang-tidy.sh to ${file}^" echo $DIR/duneclang-tidy.sh $compile_commands_dir $file $DIR/duneclang-tidy.sh $compile_commands_dir $file fi -done +done |& awk -f $(dirname $0)/duneclang-tidy_scrub_output.awk + exit 0 diff --git a/cpplint/duneclang-tidy.sh b/cpplint/duneclang-tidy.sh index 15902ea46..d48435675 100755 --- a/cpplint/duneclang-tidy.sh +++ b/cpplint/duneclang-tidy.sh @@ -267,9 +267,9 @@ for source_file in $source_files; do echo echo "=========================Checking $source_file=========================" - clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: size_t;ptrdiff_t;size_type;difference_type}]}" -header-filter=.* $source_file |& awk -f $(dirname $0)/duneclang-tidy_scrub_output.awk + clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: size_t;ptrdiff_t;size_type;difference_type}]}" -header-filter=.* $source_file -done +done #echo "Deleting $tmpdir/compile_commands.json" rm -f $tmpdir/compile_commands.json diff --git a/cpplint/duneclang-tidy_scrub_output.awk b/cpplint/duneclang-tidy_scrub_output.awk index 99c4f7b38..815be02a6 100644 --- a/cpplint/duneclang-tidy_scrub_output.awk +++ b/cpplint/duneclang-tidy_scrub_output.awk @@ -13,11 +13,6 @@ BEGIN { next } - # ...and catch instances of external header errors where the awk record includes the opening blurb - - if (NR == 1 && $0 ~ /Error while processing/ && $0 ~ /\/cvmfs/) { - next - } # Get rid of complaints about moo-generated code if ($0 ~/\/codegen\//) { @@ -72,5 +67,22 @@ BEGIN { next } + # Catch instances of external header errors where the awk record includes the opening blurb + + if ($0 ~ /Error while processing/ && $0 ~ /\/cvmfs/) { + next + } + + match($0, /[[:alnum:]]+.h[px][px]:[0-9]+:[0-9]+/) + + repeat = 0 + if (RLENGTH != -1) { + header_line_and_loc = substr($0, RSTART, RLENGTH) + if (header_line_and_loc in header_complaints) { + next + } + header_complaints[header_line_and_loc] = 1 + } + print } From 74194f493dc0750c1c7b3a7b5227ee75f157f140 Mon Sep 17 00:00:00 2001 From: John Freeman Date: Fri, 27 May 2022 14:40:27 -0500 Subject: [PATCH 2/4] JCF: various refinements to clang-tidy output based on my experience linting dfmodules; details in comments --- cpplint/duneclang-tidy.sh | 6 ++--- cpplint/duneclang-tidy_scrub_output.awk | 29 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/cpplint/duneclang-tidy.sh b/cpplint/duneclang-tidy.sh index d48435675..d5b0409c0 100755 --- a/cpplint/duneclang-tidy.sh +++ b/cpplint/duneclang-tidy.sh @@ -103,7 +103,7 @@ if [[ "$retval" != "0" ]]; then fi fi -# Left out: +# Some of the warnings/errors left out: # misc-non-private-member-variables-in-classes: since clang-tidy bizarrely includes this complaint for structs @@ -207,7 +207,7 @@ google-runtime-int,\ google-runtime-operator,\ hicpp-exception-baseclass,\ hicpp-multiway-paths-covered,\ -misc-no-recursion,\ +#misc-no-recursion,\ misc-unconventional-assign-operator,\ modernize-make-shared,\ modernize-make-unique,\ @@ -267,7 +267,7 @@ for source_file in $source_files; do echo echo "=========================Checking $source_file=========================" - clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: size_t;ptrdiff_t;size_type;difference_type}]}" -header-filter=.* $source_file + clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: size_t;ptrdiff_t;size_type;difference_type}, {key: performance-move-const-arg.CheckTriviallyCopyableMove, value: false}]}" -header-filter=.* $source_file done diff --git a/cpplint/duneclang-tidy_scrub_output.awk b/cpplint/duneclang-tidy_scrub_output.awk index 815be02a6..a2fe9a051 100644 --- a/cpplint/duneclang-tidy_scrub_output.awk +++ b/cpplint/duneclang-tidy_scrub_output.awk @@ -73,6 +73,35 @@ BEGIN { next } + # JCF, May-27-2022: there's a phenomenon where two warnings will appear in the same record, and the first is actually + # the last (unwanted) performance-unnecessary-value-param warning at the end of an ERS line + + if ($0 ~ /\[performance-unnecessary-value-param\].*\[[[:alnum:]-]+\]/) { + match($0, /\[performance-unnecessary-value-param\].*\n/) + printf("\n%s", substr($0, RSTART+RLENGTH)) + next + } + + # JCF, May-27-2022: a frequent idiom is to use bind to register a + # member function as a callback (with "this" as the bound + # variable); this strikes me as easier for both the programmer and + # the reader than the lambda function clang-tidy recommends + + if ($0 ~ /\[modernize-avoid-bind\].*,[[:space:]]*this[[:space:]]*,/ ) { + next + } + + # JCF, May-27-2022: clang-tidy's good at catching situations where + # using auto would be helpful, but we don't want it to complain if + # someone skipped auto to make it clear what the type passed to a + # templated function is. For example, + # serialization::deserialize(trigger_record_bytes); + + if ($0 ~ /\[modernize-use-auto\].*[[:alnum:]]+<[[:alnum:]]+>\(/ ) { + next + } + + # Don't have header linting repeated match($0, /[[:alnum:]]+.h[px][px]:[0-9]+:[0-9]+/) repeat = 0 From 2c8814faf94c72b0fd46b87142cdd87b21e6b709 Mon Sep 17 00:00:00 2001 From: John Freeman Date: Wed, 1 Jun 2022 09:30:21 -0500 Subject: [PATCH 3/4] JCF: get rid of clang-tidy's cppcoreguidelines-pro-bounds-pointer-arithmetic because we don't have std::span in our toolkit (yet) --- cpplint/duneclang-tidy.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpplint/duneclang-tidy.sh b/cpplint/duneclang-tidy.sh index d48435675..447e1ecfa 100755 --- a/cpplint/duneclang-tidy.sh +++ b/cpplint/duneclang-tidy.sh @@ -111,6 +111,9 @@ fi # ("if you explicitly define one of the five special member functions, # you must define them all") +# cppcoreguidelines-pro-bounds-pointer-arithmetic: since we use C++17 and not C++20 (Jun-1-2022) we don't have access +# to std::span making it very difficult to live without subscripting arrays + musts="bugprone-assert-side-effect,\ bugprone-copy-constructor-init,\ bugprone-infinite-loop,\ @@ -139,7 +142,7 @@ cppcoreguidelines-macro-usage,\ cppcoreguidelines-narrowing-conversions,\ cppcoreguidelines-no-malloc,\ cppcoreguidelines-pro-bounds-constant-array-index,\ -cppcoreguidelines-pro-bounds-pointer-arithmetic,\ +#cppcoreguidelines-pro-bounds-pointer-arithmetic,\ cppcoreguidelines-pro-type-const-cast,\ cppcoreguidelines-pro-type-cstyle-cast,\ cppcoreguidelines-pro-type-reinterpret-cast,\ From 31586373d893c12049449682b7856a87e33b0d2f Mon Sep 17 00:00:00 2001 From: John Freeman Date: Thu, 2 Jun 2022 13:18:19 -0500 Subject: [PATCH 4/4] JCF: ignore conversions from "unsigned int" to int, etc. Continue to lint conversions from "unsigned long", uint32_t, as these are typically chosen to not be mere int substitutes --- cpplint/duneclang-tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpplint/duneclang-tidy.sh b/cpplint/duneclang-tidy.sh index 4769d5b1c..a6f249068 100755 --- a/cpplint/duneclang-tidy.sh +++ b/cpplint/duneclang-tidy.sh @@ -270,7 +270,7 @@ for source_file in $source_files; do echo echo "=========================Checking $source_file=========================" - clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: size_t;ptrdiff_t;size_type;difference_type}, {key: performance-move-const-arg.CheckTriviallyCopyableMove, value: false}]}" -header-filter=.* $source_file + clang-tidy -extra-arg=-ferror-limit=0 -p=$tmpdir -checks=${musts},${maybes} -config="{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: unsigned;size_t;ptrdiff_t;size_type;difference_type}, {key: performance-move-const-arg.CheckTriviallyCopyableMove, value: false}]}" -header-filter=.* $source_file done