search

AMReX-Combustion / PeleLMeX

An adaptive mesh hydrodynamics simulation code for low Mach number reacting flows without level sub-cycling.
https://amrex-combustion.github.io/PeleLMeX/
BSD 3-Clause "New" or "Revised" License
24 stars 32 forks source link

use regex to ignore headers from submodules for clang-tidy #308

Closed baperry2 closed 7 months ago

baperry2 commented 7 months ago

Right now we get a ton of clang-tidy warnings from sundials and amrex and just use grep to filter them out, but sometimes things get garbled and that fails.

marchdf commented 7 months ago

I hate to ask but... are you confident this works? I was using this at some point: HeaderFilterRegex: ".+/PeleLMeX/Source/.*\\.H" but I wasn't 100% that it was not excluding stuff I didn't want (I know for sure that mine didn't check the Exec folder). I do think this is worth figuring out so I hope yours works better than mine !

baperry2 commented 7 months ago

I did test, but not for Exec. I added that to the regex, as well as dummy code that should introduce warnings with Source, Exec, and PelePhysics to verify that these get caught. Presuming the clang-tidy tests here fail with the correct 3 warnings, I think this is ready to go (after removing the dummy code)

marchdf commented 7 months ago

nicely done! We need this for PeleC as well. Can you remove the grep now from the CI file? Also, does this make things faster?

baperry2 commented 7 months ago

There isn't a significant impact on runtime, the main advantage is just getting 200MB of junk out of the log files

The grep is still needed because a handful of warnings still come through from AMReX

PeleLMeX/Submodules/amrex/Src/Extern/SUNDIALS/AMReX_NVector_MultiFab.cpp:586:25: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]