Add some extra warnings, tested on GCC 9.4.0

[paulharris]

For resolving bug #7200 not sure how I'm supposed to link this to the bug more directly, is that correct?

Added: -Wmissing-declarations -Wzero-as-null-pointer-constant

Added a lot of 'static' declarations in front of functions, to satisfy -Wmissing-declarations

Adjusted the CPP code generator to output nullptr where appropriate, to satisfy -Wzero-as-null-pointer-constant

---

Note that I did NOT run sh scripts/clang-format-git.sh as that introduced changes not relevant to this PR.

Also note that I haven't tested on any compilers except GCC 9.4.0

And I'm not sure if I've put the new warning flags in the correct place in CMakeLists.txt I am not sure, but I think -Wzero-as-null-pointer-constant was only added in more modern GCC compilers, not sure what version, but this is why I put it in the > 7.0 section.

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

[paulharris]

I think I'll need you to fixup the benchmarks test. It looks like there is a bunch of 3rd party code (like googletest) that might need those flags disabled for the benchmark part of the compile? Or, should those bits of code be fixed up? I'm not sure, I don't use that code so I'm not sure how you'd want it to be done.

[dbaileychess]

I think we need to set these flags to be private to the target (see https://cmake.org/cmake/help/latest/command/target_compile_options.html) instead of setting them in the global C++ CMAKE_CXX_FLAGS. That way they don't propagate to other targets (such as googletests). This was on my radar as something to clean up, but it seems like we need to do it now.

[dbaileychess]

It would be great if you wanted to move us over to that way, if not, I can prioritize switching over to using localized arguments.

[paulharris]

Sorry I'm new to that localized argument feature myself, are you able to handle that please?

[dbaileychess]

Ok, give me a few days to find time to get it in. I'll ping you when you can rebase on top.

[dbaileychess]

Just letting you know I'm currently working on this in #7222

[dbaileychess]

@paulharris This PR can be started again. There should be a single location in the CMakeLists.txt to add these warnings too, and they shouldn't propagate to other targets (like the Benchmarks)

[paulharris]

Give this a try. Thanks, I learned something new about cmake, nice new feature for dealing with compiler flags!

[dbaileychess]

Looks good.

If you want, you could group all the static functions and place them at the head of the file within an anonymous namespace as well, as that it limits the scope to the compilation unit. This can be done in a separate PR.

[paulharris]

Mind if I do the static --> namespace in this PR ?

[dbaileychess]

That's fine to do the anonymous namespace.

[paulharris]

@dbaileychess please review and consider merging :) I finally had a chance to cycle back to finish this up.

[paulharris]

Can we also tag a new release? v2.0.7 ? I'd like to update the recipe in conan while I'm working on flatbuffers.

[paulharris]

@dbaileychess how is this?

[dbaileychess]

Thanks! Look forward to the follow up PR.