Cleanup warnings in current vtk 9 plans?

Hi, so I am currently building vtk 9.2 from sources and noted a lot of warnings (>1000 warnings in release mode when building with vs2022 !) during the build are happening, most often due to unferenced formal parameters.

This is a common issue that has many common solutions, but because we can’t assume MSVC or even c++17 standard is used (if it were we could use [[maybe_unused]] ), it could be still solved by using a macro similar to the the one in MSVC called UNREFERENCED_PARAMETER():

#define VTK_UNREFERENCED_PARAMETER(x) x

then simply use it it in (typically callback or assert) code like this:

VTK_UNREFERENCED_PARAMETER(my_unreferenced_param);

Would the VTK team welcome a contribution there?
If yes, what would be the best place to add such a macro in the toolkit?

EDIT: note that the idea in cleaning up those warnings as well is:

  1. Now we might pay more attention to potentially more important warnings (such as unsigned/signed ones or possible loss of data after implicit conversions that also happen currently see my second message down below)
  2. We may realize that some parameters should be used in some rare cases…

Here are the most frequent warnings codes list after building a release build of an official 9.2.2 (custom build with few extra non standard modules I can detail if needed but no code changes yet) with vs2022:

You can fork the VTK repository here https://gitlab.kitware.com/vtk/vtk and submit a merge quest.

Thanks and would certainly like to start to participate on these,
but due to the repetitive nature of some warnings,
I would need first some buy-in to an approach so that I don’t waste VTK team or my time reviewing these changes.

So concretely, and starting with the aforementioned warning I would gladly volunteer on starting helping with, would I get more buy-in in how to fix this unreferenced parameter warning to maximize the chance of a successful merge request?

You could post an issue on the VTK gitlab site for discussion or submit a cut-down MR with just some preliminary changes and ask for review.

1 Like

Good idea,
see:
https://gitlab.kitware.com/vtk/vtk/-/issues/18688

Hi @fab672000

We are not testing vs2022 yet, so that may explains the warnings, did you try with vs2019 ?

We do not allow warnings in our CI and hide the one that we cant resolve (they still are a few, but definitely not a thousand).

Best,

1 Like

vtkNotUsed aleady exists for unreferenced parameters.

3 Likes

OK will test and come back to you on these.

OK great, so if we can show valid cases, contributions should be even easier

Thanks to both of you, I’m new to vtk but not new in C++ development and willing to help :slight_smile:

2 Likes

You can find hundreds of example usage in VTK, here is one: https://gitlab.kitware.com/vtk/vtk/-/blob/master/Filters/Extraction/vtkExtractParticlesOverTime.cxx#L176

1 Like

Awesome, glad to have you contribute!

1 Like

OK so in vs2019, we have similarly 1117 warnings, now I looked in more details and it appears that 1095 warnings are caused by third party code that I must probably include when using modules related to STL models imports.

Then, there are still approx 20 warnings, most of them seem related to DIY, ParallelDIY2 & IOSS

  C:\src\Depot\main\COTS\vtk\source\Common\Core\Testing\Cxx\TestLoggerDisableSignalHandler.cxx(30): warning C4702: unreachable code [C:\src\Depot\main\COTS\vtk\bld\Common\Core\Testing\Cxx\TestLoggerDisableSignalHandlerCxx.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\src\Depot\main\COTS\vtk\source\Common\Core\vtkMath.h(1892): warning C4723: potential divide by 0 [C:\src\Depot\main\COTS\vtk\bld\Common\Core\Testing\Cxx\vtkCommonCoreCxxTests.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\src\Depot\main\COTS\vtk\source\Common\Core\vtkMath.h(1892): warning C4723: potential divide by 0 [C:\src\Depot\main\COTS\vtk\bld\Common\Core\Testing\Cxx\vtkCommonCoreCxxTests.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Parallel\DIY\vtkDIYDataExchanger.cxx) [C:\src\Depot\main\COTS\vtk\bld\Parallel\DIY\ParallelDIY.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Parallel\DIY\vtkDIYUtilities.cxx) [C:\src\Depot\main\COTS\vtk\bld\Parallel\DIY\ParallelDIY.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Parallel\DIY\vtkDIYGhostUtilities.cxx) [C:\src\Depot\main\COTS\vtk\bld\Parallel\DIY\ParallelDIY.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\Extraction\vtkExpandMarkedElements.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\Extraction\FiltersExtraction.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\src\Depot\main\COTS\vtk\source\IO\IOSS\vtkIOSSModel.cxx(634,17): warning C4189: 'nodeCount': local variable is initialized but not referenced [C:\src\Depot\main\COTS\vtk\bld\IO\IOSS\IOIOSS.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\src\Depot\main\COTS\vtk\source\IO\IOSS\vtkIOSSModel.cxx(630,21): warning C4189: 'elementCount': local variable is initialized but not referenced [C:\src\Depot\main\COTS\vtk\bld\IO\IOSS\IOIOSS.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkAdaptiveResampleToImage.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkExtractSubsetWithSeed.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<float>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkDIYKdTreeUtilities.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkGenerateGlobalIds.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkGhostCellsGenerator.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkOverlappingCellsDetector.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkPResampleToImage.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkPResampleWithDataSet.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]
  C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.33.31629\include\xmemory(682,47): warning C4996: 'vtkdiy2::Bounds<long>::Bounds': "Default Bounds constructor should not be used; old behavior is preserved for compatibility. Pass explicitly the dimension of the Bounds instead." (compiling source file C:\src\Depot\main\COTS\vtk\source\Filters\ParallelDIY2\vtkProbeLineFilter.cxx) [C:\src\Depot\main\COTS\vtk\bld\Filters\ParallelDIY2\FiltersParallelDIY2.vcxproj] [C:\src\Depot\main\COTS\vtk\vtk.proj]

Starting with baby steps and analysis on the reminders, prioritizing the non 3rd party ones first:

  1. Now for vtkMath.h:1892: warning C4723: potential divide by 0
    I don’t see any problem as the range 0,1 equality is tested in a pre-condition in the code before.
    see:
if (range[0] == range[1])
  {
    result = 0.0;
  }
  else

Suggesting to add a pragma to remove this unjustified warning in MSVC, thoughts?

  1. But for vtkIOSSModel.cxx warnings for these are real and :
    unused parameters:
    const int64_t elementCount = pair.second;
    const int nodeCount = element->number_nodes();

suggesting to remove these unused variables.
If asserts are needed they should be added later.

  1. TestLoggerDisableSignalHandler.cxx:
    using the void main() construction instead of int main() would be solving the problem, eventually a pragma to disable the warning could do the same thing as after abort() no return statement would be executing.

Most of these warnings (outside the third party sub directory) have been already either directly or indrectly fixed on master.
Lesson learned: master is about 6 months more recent than latest release branch, so always double check with master first.

There are other discussions for a potential strategy on the third party warnings when building vtk (they could be disabled in the vtk build and kept enabled upstream as an example.

You may want to read this:
https://gitlab.kitware.com/vtk/vtk/-/tree/master/Documentation/dev

and this

https://gitlab.kitware.com/vtk/vtk/-/tree/master/Documentation/dev/git

1 Like

I actually used your documentation for creating and merging topic and that was straight forward, good doc! Did not need much on git that I already use regularly on Github but the latest dev updates from Ben are very interesting and helpful.

1 Like

Hello,

The excess warnings coming from VTK causes my IDE to crash, so I disable them by adding

    gcc:QMAKE_CXXFLAGS += -isystem $$_VTK_INCLUDE  #This is to suppress the countless compiler warnings from VTK headers
  clang:QMAKE_CXXFLAGS += -isystem $$_VTK_INCLUDE  #This is to suppress the countless compiler warnings from VTK headers
  mingw:QMAKE_CXXFLAGS += -isystem $$_VTK_INCLUDE  #This is to suppress the countless compiler warnings from VTK headers
msvc:QMAKE_CXXFLAGS += /external:I $$_VTK_INCLUDE  #This is to suppress the countless compiler warnings from VTK headers

to the program’s qmake.pro (my program is built with qmake) configuration script. It’s likely you can do a similiar thing in CMakeLists.txt, Makefile, or whatever configuration system you may be using to build your program. Recall that you add such flags to the configuration of your program, not VTK’s.

So, in MSVC, you add /external:I <VTK include path> as a compiler flag to tell the compiler that VTK is a system library and it is safe to ignore warnings coming from them. You can also do this to possible other sources of copious compiler warnings.

I hope this helps,

Paulo

Very interesting Paul thanks!
It may be applicable to some extent: e.g. in the 3rd party libs which would be acceptable for me to disable (and only those not the vtk core warnings in my case) with an appropriate new cmake option I am proposing currently, see this issue

Adding to the discussion, I use the MinGW64 compiler. So this is not MSVC’s sole fault. I usually get around 50k warnings from VTK, depending on which .cpp files I change, which not only crash the IDE, they also clutter the log files in the CI/CD platform.

Actually, you might find that when you filter out the 3rd party library warnings (as I did in MSVC, probably very similar in MinGW) , there are very few warnings caused by the actual VTK code, it’s pretty clean out there.

Which is why I suggested an approach making possible to disable as a cmake option the 3rd party noise.

Please also note that there is already an option to disable these to appear in cdash (search for cdash in advanced cmake options in cmake-gui), but unfortunately that does not solve the problem of those like us that have their own CI software.