CI: update tidy image #14589

tjhei · 2022-12-19T20:26:10Z

This updates to clang 14 from 10.

Also updates cmake to 3.22.1 @tamiko

masterleinad · 2022-12-19T23:35:04Z

manifest for tjhei/candi-base:clang14 not found.

contrib/ci/Jenkinsfile.tidy

masterleinad · 2022-12-20T16:57:34Z

/rebuild

masterleinad · 2022-12-20T17:01:16Z

make sure clang, clang++, and run-clang-tidy.py (part of clang) are in the path

tjhei · 2022-12-23T00:31:42Z

I am struggling to make configuration work with clang 14 and MPI (so we can get MPI warnings).
The only thing that configures correctly for me is

export MPICH_CXX=clang++
export MPICH_CC=clang
CC=mpicc CXX=mpicxx cmake <BLA>

But this leads to errors that clang-tidy can not find <mpi.h>.
If I remove CXX=mpicxx or set it to clang++, deal.II doesn't configure with MPI. :-(

tamiko · 2023-01-06T20:09:54Z

@tjhei I just checked the docker image: The problem is that you set CC=mpicc and CXX=mpicxx in the docker environment. With that the FindMPI.cmake wrapper fails in funny ways. What works for me is to:

unset CC
unset CXX
cmake -DMPI_EXECUTABLE_SUFFIX=".mpich" -DWITH_MPI=ON -GNinja ...

The issue that remains is that the FindMPI.cmake wrapper does not work with clang ??!!

tamiko · 2023-01-06T21:51:12Z

@tjhei I took the liberty to update this branch with a workaround.

tamiko · 2023-01-06T22:12:21Z

@tjhei I don't understand what's going on here. The docker image contains clang++ and run-clang-tidy.py:

» singularity@singularity ~ % singularity run docker://tjhei/candi-base:clang14
Getting image source signatures
Writing manifest to image destination
Storing signatures
INFO:    Creating SIF file...

Singularity> which run-clang-tidy.py
/usr/bin/run-clang-tidy.py
Singularity> which clang++
/usr/bin/clang++

And running the run_clang_tidy.sh by hand works. Does the Jenkins worker by chance cache a corrupt docker image?

tamiko · 2023-01-06T22:25:26Z

contrib/utilities/run_clang_tidy.sh

@@ -41,9 +41,13 @@ if test ! -d "$SRC/source" -o ! -d "$SRC/include" -o ! -d "$SRC/examples" -o ! -
 fi
 echo "SRC-DIR=$SRC"

+# unset CC and CXX
+unset CC
+unset CXX


~~@tjhei Completely unsetting CC and CXX fixes the configure issue. clang-tidy afterwards seems to be smart enough to accept the fact that the project had been configured with gcc.~~

~~Alternatively, if you prefer, you can also~~ add back the compiler wrappers (i.e. remove the two unset) and specify -DMPI_INCLUDE_DIRS=/usr/include/x86_64-linux-gnu/mpich as an option. This seems to work as well.

@tjhei The current variant now uses the compiler wrappers + sets MPI_INCLUDE_DIRS manually to make clang-tidy happy.

tamiko · 2023-01-06T22:25:50Z

contrib/utilities/run_clang_tidy.sh

 # enable MPI (to get MPI warnings)
 # export compile commands (so that run-clang-tidy.py works)
-ARGS=("-D" "DEAL_II_WITH_MPI=ON" "-D" "CMAKE_EXPORT_COMPILE_COMMANDS=ON" "-D" "CMAKE_BUILD_TYPE=Debug" "$@")
+ARGS=("-D" "DEAL_II_WITH_MPI=ON" "-D" "CMAKE_BUILD_TYPE=Debug" "$@")


I have removed CMAKE_EXPORT_COMPILE_COMMANDS. We now set this unconditionally :-)

tamiko · 2023-01-07T06:47:11Z

@tjhei We will need to resolve the docker image issue, ie., whatever causes

make sure clang, clang++, and run-clang-tidy.py (part of clang) are in the path
script returned exit code 2

Furthermore, clang-tidy-14 now warns at a couple of places that we have to fix up :-)

tamiko · 2023-01-07T07:47:19Z

New warnings (54 new warnings):

include/deal.II/base/aligned_vector.h:1878:61: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/mpi.templates.h:123:31: error: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr,-warnings-as-errors]
include/deal.II/base/mpi.templates.h:83:64: error: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr,-warnings-as-errors]
include/deal.II/base/table.h:2747:35: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:2758:35: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:2769:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:2780:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:2789:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:2798:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3142:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3153:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3162:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3171:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3303:15: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3317:15: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3371:16: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/table.h:3391:16: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/matrix_free/matrix_free.h:2543:10: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/meshworker/integration_info.templates.h:190:23: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/data_out_dof_data.templates.h:1938:26: error: the const qualified variable 'name' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
include/deal.II/numerics/matrix_creator.templates.h:1010:9: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/matrix_creator.templates.h:1444:9: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:484:9: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:491:29: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:493:31: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:500:29: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:502:31: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:506:29: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_mean_value.templates.h:149:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_point_value.templates.h:306:39: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_point_value.templates.h:370:29: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_point_value.templates.h:416:39: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_point_value.templates.h:487:29: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_rhs.templates.h:197:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_rhs.templates.h:350:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_rhs.templates.h:487:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_rhs.templates.h:56:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
source/base/qprojector.cc:894:22: error: the const qualified variable 'quadrature_f' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
source/base/quadrature_lib.cc:1091:43: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
source/dofs/dof_tools.cc:2760:72: error: the const qualified variable 'cell' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
source/dofs/dof_tools.cc:2791:72: error: the const qualified variable 'cell' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
source/fe/mapping_q.cc:126:48: error: the const qualified variable 'quad_array' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
source/grid/grid_in.cc:4263:10: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
source/grid/grid_tools.cc:5046:68: error: the const qualified variable 'cell' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization,-warnings-as-errors]
source/lac/sparsity_tools.cc:624:5: error: use range-based for loop instead [modernize-loop-convert,-warnings-as-errors]
source/numerics/time_dependent.cc:931:9: error: use range-based for loop instead [modernize-loop-convert,-warnings-as-errors]
examples/step-30/step-30.cc:698:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/base/aligned_vector.h:1878:61: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/lac/full_matrix.templates.h:953:36: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/lac/full_matrix.templates.h:984:36: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/matrix_free/matrix_free.templates.h:2062:17: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:325:35: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_integrate_difference.templates.h:412:36: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]
include/deal.II/numerics/vector_tools_rhs.templates.h:350:7: error: redundant cast to the same type [google-readability-casting,-warnings-as-errors]

tamiko · 2023-01-07T07:51:20Z

160+ new warnings about "avoid repeating the return type from the declaration; use a braced ...":

include/deal.II/base/index_set.h:1585:12: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
include/deal.II/base/mpi_consensus_algorithms.h:1797:16: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
include/deal.II/base/mpi_consensus_algorithms.h:2158:16: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
include/deal.II/base/patterns.h:2103:18: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
include/deal.II/base/patterns.h:2165:16: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
...
include/deal.II/particles/property_pool.h:446:12: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
source/base/path_search.cc:167:10: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
source/base/polynomial.cc:683:14: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
source/base/quadrature_selector.cc:80:10: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]
source/fe/fe_simplex_p.cc:348:10: error: avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list,-warnings-as-errors]

stale

tamiko · 2023-01-07T07:54:51Z

550+ warnings about "C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast":

examples/step-21/step-21.cc:761:53: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
examples/step-21/step-21.cc:763:58: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
examples/step-21/step-21.cc:766:24: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
examples/step-21/step-21.cc:768:57: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
examples/step-21/step-21.cc:771:24: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
...
source/fe/fe_simplex_p_bubbles.cc:121:63: error: C-style casts are discouraged; use static_cast [google-readability-casting,-warnings-as-errors]
source/multigrid/mg_base.cc:39:7: error: C-style casts are discouraged; use static_cast (if needed, the cast may be redundant) [google-readability-casting,-warnings-as-errors]
source/multigrid/mg_transfer_internal.cc:938:49: error: C-style casts are discouraged; use static_cast (if needed, the cast may be redundant) [google-readability-casting,-warnings-as-errors]
source/multigrid/mg_transfer_internal.cc:962:23: error: C-style casts are discouraged; use static_cast [google-readability-casting,-warnings-as-errors]
source/multigrid/mg_transfer_internal.cc:962:23: error: C-style casts are discouraged; use static_cast (if needed, the cast may be redundant) [google-readability-casting,-warnings-as-errors]

Work around a bug with FindMPI.cmake not detecting the include path for mpich.

bangerth · 2023-01-18T02:23:42Z

The warnings shown above about C-style casts are quite aggravating because they are not actually of the type (type)value as I expected. For example, the ones in step-21.cc are of the kind

    std::vector<Vector<double>> old_solution_values(n_q_points,
                                                    Vector<double>(dim + 2));    *************

and the one in mg_base.cc is

  u = typename VectorType::value_type(0.);

and the one in fe_simplex_p_bubbles.cc is

    const Point<dim> centroid =
      ReferenceCells::get_simplex<dim>().template barycenter<dim>();

for which I can't even say what the cast is. In other words, these warnings are all wrong.

bangerth · 2023-01-18T02:28:48Z

And I also disagree about the brace-initializer-list in return statements. One could possibly make a point that here

inline IndexSet::IntervalIterator
IndexSet::begin_intervals() const
{
  compress();
  if (ranges.size() > 0)
    return IndexSet::IntervalIterator(this, 0);    ************
  else
    return end_intervals();
}

one might save some typing by writing

inline IndexSet::IntervalIterator
IndexSet::begin_intervals() const
{
  compress();
  if (ranges.size() > 0)
    return {this, 0};
  else
    return end_intervals();
}

instead, but (i) I don't think this is actually easier to read because I read this as "return a pair of values" rather than "call the constructor with these two arguments", and (ii) this becomes seriously difficult to read when the return type declaration of the function is a couple hundred lines away from the return statement: Then one really has no idea what the return statement actually returns without going all the way to the top to look up what type we're returning and how {this,0} will be interpreted. In other words, I don't particularly like this warning.

If we want to do anything about these cases, I'd much rather us write this as to avoid duplicating information:

inline auto               //          use 'auto'
IndexSet::begin_intervals() const
{
  compress();
  if (ranges.size() > 0)
    return IndexSet::IntervalIterator(this, 0);
  else
    return end_intervals();
}

masterleinad · 2023-01-18T04:04:25Z

@bangerth This pull request was superseded by #14649 and the two checks you were commenting on were disabled in #14649.

masterleinad · 2023-01-18T14:35:24Z

This needs some work if we decide to move the clang-tidy checker again.

bangerth · 2024-04-17T23:21:22Z

@masterleinad What should we do here? Want to rebase and see what's left?

masterleinad · 2024-04-18T12:53:08Z

@masterleinad What should we do here? Want to rebase and see what's left?

We don't use jenkins anymore for clang-tidy but GitHub Actions so I would just close but it's @tjhei's pull request.
We could look into updating the clang-tidy version in GitHub Actions but I don't know if there is a good motivation at this point in time.

bangerth · 2024-04-18T16:24:23Z

@tjhei ?

tjhei · 2024-04-19T23:54:46Z

I also don't think anything here is still needed.

tjhei added Infrastructure ready to test labels Dec 19, 2022

marcfehling previously approved these changes Dec 19, 2022

View reviewed changes

marcfehling added the Reviewed and ready to merge label Dec 19, 2022

masterleinad reviewed Dec 19, 2022

View reviewed changes

contrib/ci/Jenkinsfile.tidy Show resolved Hide resolved

tamiko previously approved these changes Dec 19, 2022

View reviewed changes

masterleinad removed the Reviewed and ready to merge label Dec 20, 2022

tamiko force-pushed the jenkins-tidy-update branch from 3007ff4 to dc81d9b Compare January 6, 2023 21:50

tamiko force-pushed the jenkins-tidy-update branch 2 times, most recently from 565e0be to 386980b Compare January 6, 2023 22:07

tamiko force-pushed the jenkins-tidy-update branch from 386980b to ddbe0bb Compare January 6, 2023 22:24

tamiko reviewed Jan 6, 2023

View reviewed changes

tamiko force-pushed the jenkins-tidy-update branch from ddbe0bb to 46d3ce3 Compare January 7, 2023 06:33

tamiko force-pushed the jenkins-tidy-update branch 6 times, most recently from 54844a3 to bb1a1e3 Compare January 7, 2023 07:31

This was referenced Jan 9, 2023

Update clang-tidy checks for recent clang versions #14649

Merged

Run clang-tidy in GitHub CI #14652

Merged

tjhei and others added 2 commits January 11, 2023 17:04

CI: update tidy image

cfa24f7

CI: manually specify mpich include path

7058821

Work around a bug with FindMPI.cmake not detecting the include path for mpich.

tjhei force-pushed the jenkins-tidy-update branch from bb1a1e3 to 7058821 Compare January 11, 2023 22:04

masterleinad marked this pull request as draft January 18, 2023 14:35

tjhei closed this Apr 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: update tidy image #14589

CI: update tidy image #14589

tjhei commented Dec 19, 2022

masterleinad commented Dec 19, 2022

masterleinad commented Dec 20, 2022

masterleinad commented Dec 20, 2022

tjhei commented Dec 23, 2022

tamiko commented Jan 6, 2023 •

edited

tamiko commented Jan 6, 2023

tamiko commented Jan 6, 2023

tamiko Jan 6, 2023 •

edited

tamiko Jan 6, 2023 •

edited

tamiko Jan 7, 2023

tamiko Jan 6, 2023

tamiko commented Jan 7, 2023

tamiko commented Jan 7, 2023 •

edited

tamiko commented Jan 7, 2023 •

edited

tamiko commented Jan 7, 2023

bangerth commented Jan 18, 2023

bangerth commented Jan 18, 2023

masterleinad commented Jan 18, 2023

masterleinad commented Jan 18, 2023

bangerth commented Apr 17, 2024

masterleinad commented Apr 18, 2024

bangerth commented Apr 18, 2024

tjhei commented Apr 19, 2024

CI: update tidy image #14589

CI: update tidy image #14589

Conversation

tjhei commented Dec 19, 2022

masterleinad commented Dec 19, 2022

masterleinad commented Dec 20, 2022

masterleinad commented Dec 20, 2022

tjhei commented Dec 23, 2022

tamiko commented Jan 6, 2023 • edited

tamiko commented Jan 6, 2023

tamiko commented Jan 6, 2023

tamiko Jan 6, 2023 • edited

Choose a reason for hiding this comment

tamiko Jan 6, 2023 • edited

Choose a reason for hiding this comment

tamiko Jan 7, 2023

Choose a reason for hiding this comment

tamiko Jan 6, 2023

Choose a reason for hiding this comment

tamiko commented Jan 7, 2023

tamiko commented Jan 7, 2023 • edited

tamiko commented Jan 7, 2023 • edited

tamiko commented Jan 7, 2023

bangerth commented Jan 18, 2023

bangerth commented Jan 18, 2023

masterleinad commented Jan 18, 2023

masterleinad commented Jan 18, 2023

bangerth commented Apr 17, 2024

masterleinad commented Apr 18, 2024

bangerth commented Apr 18, 2024

tjhei commented Apr 19, 2024

tamiko commented Jan 6, 2023 •

edited

tamiko Jan 6, 2023 •

edited

tamiko Jan 6, 2023 •

edited

tamiko commented Jan 7, 2023 •

edited

tamiko commented Jan 7, 2023 •

edited