Embracing 'nullptr'

[Minty-Meeo]

Hello. This is a PR born out of another PR of mine which aims to enable more compiler warnings for the Dolphin Emulator project. While hopefully we won't want/need every external library's headers to be warning-free once all platforms support CMake 3.25, I thought why not improve Imgui's compliance to the C++ Core Guidelines. I found hints in the comments of the examples that a switch from 'NULL' to 'nullptr' has already been attempted, though there was clearly more to do.

This PR is split into three commits. Round 1 consists of changes I was able to very quickly verify and fix by compiling an example on my Linux machine with '-Wzero-as-null-pointer-constant' enabled. Round 2 has changes I verified using context clues, online documentation, and by switching to my Windows VM for further info. Finally, I have put the changes to (most) comments and documentation in a seperate commit. I am not familiar enough with Imgui to be sure I updated the documentation correctly, so please tell me if I did not.

[Minty-Meeo]

There are a few instances where I've seen NULL being explicitly casted to a different pointer type. Are these still desired?
And I must admit, I am not situated to compile things like the Android or Apple examples. If someone would confirm I made no obvious errors with those and any others, it'd be appreciated.

[ocornut]

I am not sure I understand what you are "resolving" in the sense that we already intently disable the warnings in our code?
Is any of this actually needed for Dolphin since we have pragmas in place?

I'm ok to merge in location where those pragma are not convenient or too noisy (e.g. examples' main.cpp, or in selected places) but I am not sure there is much value doing it everywhere.

It's also not possible in the imstb_xxx files which are C code and external librairies.

It's probably incorrect in the fonts->SetTexID(0) calls as that type can be many things (compile-time defined by user) and using 0 with implicitly cast better to more types.

I'll look into selectively merging some of it.

[Minty-Meeo]

So this is a bit embarrassing. Several days ago, I had encountered warnings being generated by the implot library when included with -Wzero-as-null-pointer-constant. However, I later misremembered it as both imgui and implot causing warnings, and went through the effort of purging zero as a null pointer constant in both repositories. You are correct that the warnings are properly suppressed in the imgui headers, though I still believe fixing the warnings is a better approach. Also, sorry for accidentally touching C header files.

[Minty-Meeo]

While I was reverting my changes to ImFontAtlas::SetTexID(ImTextureID), I found an example which already ignores your advice, lol. I will still revert it for mine.

imgui/backends/imgui_impl_metal.mm

Lines 342 to 348 in a7703fe

	void ImGui_ImplMetal_DestroyFontsTexture()
	{
	ImGui_ImplMetal_Data* bd = ImGui_ImplMetal_GetBackendData();
	ImGuiIO& io = ImGui::GetIO();
	bd->SharedMetalContext.fontTexture = nil;
	io.Fonts->SetTexID(nullptr);
	}

[ocornut]

I have merged 506f7e0 to cover all backends/examples/docs/misc files but not the main codebase.
I took the liberty to force-push the remaining changes into another commit pushed here, for reference.
(pushed the small change for Metal backend as part of 9203883)

I'm still not at ease with this because:

• this will make lots of pr and private branch conflicts (I personally manage hundreds of wip patches/branches)
• i am not sure how this will behave with a variety of code parsers/generators aimed for C and other languages (that's a minor issue as I'm sure they can fix).
• we are a little bit abusive of horizontal space usage in headers already, this doesn't help.
• at heart I don't think we are solving a meaningful problem to apply this change on library side. I understand there are advantages and reasons at caller side to use nullptr (e.g. MenuItem("Foo", nullptr, nullptr) in particular is not ambiguous as per our two variants, whereas using NULL would be). But none of the changes here are having an effect on user aside from suggesting demo reader to use nullptr.

Aside from those consideration, I must say I genuinely appreciate that your PR is being well done and polished (with consideration for aligned comments).

As I general answer to:

why not improve Imgui's compliance to the C++ Core Guidelines.

I surely can tell that if were to follow any sorts of C++ guidelines from Mr Stroustrup, Dear ImGui wouldn't exist :-)

[Minty-Meeo]

I hate to be a bother, but is there any way you can also help along the sister PR to this one over on Implot, @ocornut? I noticed you appear to be a collaborator.

[ocornut]

I don’t want to overstep on decision at implot but in the meanwhile i would accept one that just disabled the warning like imgui does. Update done on implot.