InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

Wmissing variable declaration #4676

Closed andrei-sandor closed 1 month ago

andrei-sandor commented 1 month ago

COMP: Fix missing declarations of unused variables.

This is to fix the -Wmissing-variable-declarations warning of clang.

This is simply related that the variable in question is not used in the file


COMP: Fix missing variables declaration (static)

This is a fix about -Wmissing-variable-declarations warning of clang

This is about the static case. A lot of variables were marked as warning due to non declaration. The static keyword was added when it makes sense that the variable is used internally.

seanm commented 1 month ago

...proper formatting has to be followed to satisfy the CI.

Yeah, we haven't found a way to install old clang-format version 8, so we keep skipping the commit hooks. And newer clang-format makes unrelated changes. There's no magic "Do: reformat" command like with VTK's gitlab?

dzenanz commented 1 month ago

Normally, ITK builds the correct version of clang-format, and puts it e.g. here: C:\Dev\ITK-vs22\ClangFormat-prefix\src\ClangFormat\clang-format.exe. You should be able to copy that executable into a more suitable location, if you like. But also pre-commit hooks should be using it?

seanm commented 1 month ago

Normally, ITK builds the correct version of clang-format, and puts it e.g. here: C:\Dev\ITK-vs22\ClangFormat-prefix\src\ClangFormat\clang-format.exe. You should be able to copy that executable into a more suitable location, if you like. But also pre-commit hooks should be using it?

I have ITK_USE_CLANG_FORMAT=ON but I don't find any such thing built. That gets built in the bin directory?, or do you have to make install?

sean@hobgoblin ITK-bin % find . -iname "*clang*"
./KWStyle/Utilities/boost/config/compiler/clang_version.hpp
./KWStyle/Utilities/boost/config/compiler/clang.hpp
./KWStyle/Utilities/boost/predef/compiler/clang.h
./ITKClangFormatConfig.cmake
dzenanz commented 1 month ago

Here is the output on my Linux build:

dzenan@corista:~/ITK-git-rel$ find . -iname "clang*"
./temp/clang-format-Linux
./Wrapping/Generators/CastXML/castxml/share/castxml/clang
./Wrapping/Generators/CastXML/castxml-prefix/src/castxml/share/castxml/clang
./clang-format-Linux
./ClangFormat-prefix
./ClangFormat-prefix/src/ClangFormat-stamp
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-update
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-download-err.log
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-download-Release-impl.cmake
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-patch
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-mkdir
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-done
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-install
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-download-out.log
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-configure
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-patch-info.txt
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-download
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-build
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-urlinfo.txt
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-update-info.txt
./ClangFormat-prefix/src/ClangFormat-stamp/ClangFormat-download-Release.cmake
./ClangFormat-prefix/src/ClangFormat
./ClangFormat-prefix/src/ClangFormat/clang-format
./ClangFormat-prefix/src/ClangFormat-build
./ClangFormat-prefix/tmp/ClangFormat-cfgcmd.txt
./ClangFormat-prefix/tmp/ClangFormat-mkdirs.cmake
./KWStyle/Utilities/boost/config/compiler/clang.hpp
./KWStyle/Utilities/boost/predef/compiler/clang.h
./CMakeFiles/ClangFormat.dir
./CMakeFiles/ClangFormat-complete
dzenan@corista:~/ITK-git-rel$
dzenanz commented 1 month ago

Yes, it gets build in the bin directory, under name clang-format-Linux (see above listing). It should be similar for Mac.

seanm commented 1 month ago

Yes, it gets build in the bin directory, under name clang-format-Linux (see above listing). It should be similar for Mac.

I've just built with VERBOSE=1 make and tried searching for keywords like clang and format but not found any evidence of it trying to build that. Do you have an exact string I might search for to see what's (not) happening?

dzenanz commented 1 month ago

Target is called ClangFormat, so maybe try make ClangFormat? Here are the relevant entries from my cache:

Screenshot 2024-05-17 16 52 02
seanm commented 1 month ago
sean@hobgoblin ITK-big-bin % make ClangFormat
make: *** No rule to make target `ClangFormat'.  Stop.

But grepping ClangFormat in the ITK codebase I found this URL: https://data.kitware.com/api/v1/file/5d274e88877dfcc902effc47/download which appears to be the Mac clang-format8 executable! so I just downloaded that, copied it somewhere, did chmod +x and then git config clangFormat.binary /Users/sean/Downloads/clang-format8 and presto the ITK git hooks are now happy!

andrei-sandor commented 1 month ago

@hjmjohnson Yes sure, that's good for me.