Enabling Warnings for VisIt

[JustinPrivitera]

This repo has lots of good things: https://github.com/cpp-best-practices/cppbestpractices/tree/master

In particular, https://github.com/cpp-best-practices/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang
has the full list of warnings.

To enable all of these warnings, I added the following to the cmake invocation in recmake_visit.sh:
-DCMAKE_CXX_FLAGS:STRING=" -Wall -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align -Wunused -Woverloaded-virtual -Wpedantic -Wconversion -Wsign-conversion -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wnull-dereference -Wuseless-cast -Wdouble-promotion -Wformat=2 -pedantic -m64 -fPIC" -DCMAKE_C_FLAGS:STRING=" -Wall -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align -Wunused -Woverloaded-virtual -Wpedantic -Wconversion -Wsign-conversion -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wnull-dereference -Wuseless-cast -Wdouble-promotion -Wformat=2 -pedantic -m64 -fPIC"
Then I ran ./recmake_visit.sh.

If you do this and then build, you will see tens of thousands of warnings, including many from our third party dependencies. If we're just trying to view warnings for one particular file, we can do the following to hide all other warnings:

// place these before any includes
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wall"
#pragma GCC diagnostic ignored "-Wextra"
#pragma GCC diagnostic ignored "-Wshadow"
#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
#pragma GCC diagnostic ignored "-Wold-style-cast"
#pragma GCC diagnostic ignored "-Wcast-function-type"
#pragma GCC diagnostic ignored "-Wcast-align"
#pragma GCC diagnostic ignored "-Wunused"
#pragma GCC diagnostic ignored "-Woverloaded-virtual"
#pragma GCC diagnostic ignored "-Wpedantic"
#pragma GCC diagnostic ignored "-Wconversion"
#pragma GCC diagnostic ignored "-Wsign-conversion"
#pragma GCC diagnostic ignored "-Wfloat-conversion"
#pragma GCC diagnostic ignored "-Wmisleading-indentation"
#pragma GCC diagnostic ignored "-Wduplicated-cond"
#pragma GCC diagnostic ignored "-Wduplicated-branches"
#pragma GCC diagnostic ignored "-Wlogical-op"
#pragma GCC diagnostic ignored "-Wnull-dereference"
#pragma GCC diagnostic ignored "-Wuseless-cast"
#pragma GCC diagnostic ignored "-Wdouble-promotion"
#pragma GCC diagnostic ignored "-Wformat=2"
#pragma GCC diagnostic ignored "-Wpragmas"
#pragma GCC diagnostic ignored "-pedantic"
#pragma GCC diagnostic ignored "-Wunused-parameter"

// includes here
...

// after includes
#pragma GCC diagnostic pop

This should hide almost everything, although macros that depend on other things can still cause warnings to appear for code outside the file you are editing. But this mostly hides everything and makes it manageable.

[JustinPrivitera]

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

[markcmiller86]

Are you making a pitch for trying to get our warnings down to zero?

If so, I am on board with that.

It will be a lot (a very lot) of work but well worth it I think.

At one point in the distant past, I tried to add a Subversion hook to prevent checkins that increased existing warning counts in any committed file. In other words, there could be warnings but no code could be committed that would increase their number. Over time, that in theory should force the code base towards zero warnings but only if all source files get routinely visited. In VisIt they don't 😉

[JustinPrivitera]

Are you making a pitch for trying to get our warnings down to zero?

I think it would be very good indeed if out warnings went down to zero (or as close to zero as is reasonable). But it also looks like it would be a huge undertaking. Before I turned on all those pragma statements up above, I saw what must have been tens of thousands of warnings. Scrolling up to the very top of the terminal window didn't even show the beginning of them. We also get a lot of warnings from third-party dependencies, which we can't do much about. But if we made a concerted effort to squash the ones in VisIt then I think that would be great. Maybe in addition to doc hackathons we could have warning hackathons 🙃

At one point in the distant past, I tried to add a Subversion hook to prevent checkins that increased existing warning counts in any committed file. In other words, there could be warnings but no code could be committed that would increase their number. Over time, that in theory should force the code base towards zero warnings but only if all source files get routinely visited. In VisIt they don't 😉

If there is a way to make git reject commits that add to the warning count, that would be cool.

I will spend more time in the future fixing warnings, so long as nobody complains that I'm not working on other things.

[markcmiller86]

Maybe we should have a few warning hack-a-thons like we do doc sprints.

[JustinPrivitera]

Assuming this is fixed (#19001) this process will be much easier.

[biagas]

You can also use target_compile_options for a specific target to specify extra warnings for only a single target (and all its files):

target_compile_options(visit_vtk PRIVATE -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align
     -Wunused -Woverloaded-virtual -Wpedantic -Wconversion -Wsign-conversion -Wmisleading-indentation
     -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wnull-dereference -Wuseless-cast
     -Wdouble-promotion -Wformat=2 -pedantic -m64 -fPIC)

You can start with as few or as many warnings you like, once you change them and run make cmake will automatically regenerate that target's build files with the new flags.