Evaluation of warnings raised by VS 2019 with `/W4`

[akien-mga]

Godot version

4.0.beta (14e1f36)

System information

Windows 10 64-bit, VS 2019, MSVC 14.29.30133

Issue description

I made some fixes and Godot's master branch should now build without warnings when using /W3 with VS 2019 (at least tools=yes target=debug build, optimized builds might find other unfixed warnings with /W3).
It's worth noting that our warnings=all preset which sets /W3 also disables /wd4267 /wd4244 /wd4305 /wd4018 /wd4800 as non-essential warnings.

I gave a try at using /W4 without disable the above non-essential warnings, here are the results:

369,102 warnings raised.

Oh boy.

Let's categorize them and see which ones are worth investigating/fixing and which ones should be ignored (or if we should outright give up on /W4 if it doesn't catch any useful warning compared to /W3).

Here's the list of unique warning codes raised and the number of occurrences for each:

Code	Occurrences	Warning Level	Message	Comment
C4100	220,816	4	'identifier': unreferenced formal parameter	Doesn't play nice with polymorphism, should be ignored. Silenced by #66595
C4127	966	4	conditional expression is constant	Fixed partially by #66583
C4201	8,487	4	nonstandard extension used: nameless struct/union	Only relevant for C89 compat, we should ignore it. Silenced by #66595
C4244	51,640	2 and 3/4	'conversion' conversion from 'type1' to 'type2', possible loss of data	Ignored in warnings={all,moderate}
C4245	352	4	'conversion' : conversion from 'type1' to 'type2', signed/unsigned mismatch	Silenced by #66595
C4267	1203	3	'var' : conversion from 'size_t' to 'type', possible loss of data	Ignored in warnings={all,moderate}
C4305	18,164	1	'context' : truncation from 'type1' to 'type2'	Ignored in warnings={all,moderate}
C4324	12	4	'struct_name' : structure was padded due to __declspec(align())	Silenced in #66545
C4389	2	4	'equality-operator' : signed/unsigned mismatch	Fixed by #66545
C4456	3	4	declaration of 'identifier' hides previous local declaration	Fixed by #66545
C4458	67,374	4	declaration of 'identifier' hides class member	66,047 occurrences fixed by #66539, 1,330 remain
C4459	1	4	declaration of 'identifier' hides global declaration	Fixed by #66545
C4701	17	4	Potentially uninitialized local variable 'name' used	Fixed by #66544 and #66548
C4702	26	4	unreachable code	Fixed by #66543 and #66568
C4703	2	4	Potentially uninitialized local pointer variable 'name' used	Fixed by #66548
C4706	37	4	assignment within conditional expression	Fixed by #66542 and #66545

For each of the above, we should assess some of the situations in which the warnings are raised and see if they're worth fixing. The warnings emitted tens of thousands of time typically come from core templates and get re-emitted everytime the template is used.

I'll post some details on each warning in follow-up comments.

Here's the full build log (7z compressed and then zipped so it can be attached, unzipped it's 59MB):

build-W4.log.7z.zip

Edit: Edited log with Unix line endings, attempting to fix some of the scrambled lines where multiple warnings would be listed on the same line, or paths would get truncated:

build-W4-unix.log.7z.zip

Some remarks:

• Looks like C4018 which we ignore with warnings=all is not emitted with a plain /W4 run. Might be that we fixed it and we can stop ignoring it.
• Similarly, C4800 which we ignore is not emitted. According to the docs it has a different meaning in VS <= 2015 and VS >= 2019 and is not emitted in VS 2017. We only support VS >= 2017 so we could un-ignore it too.

Steps to reproduce

• VS 2019, MSVC 14.29.30133
• scons p=windows CCFLAGS="/W4"

Minimal reproduction project

No response

[akien-mga]

Checking some of the warnings with low occurrence counts first.

C4324: 'struct_name' : structure was padded due to __declspec(align())

12 occurrences, actually 4 times this set:

C:\Users\Remi\Documents\Godot\godot.git\thirdparty\embree\include\embree3\rtcore_common.h(236): warning C4324: 'RTCPointQuery': structure was padded due to alignment specifier
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\embree\include\embree3\rtcore_common.h(285): warning C4324: 'RTCPointQueryContext': structure was padded due to alignment specifier
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\embree\include\embree3\rtcore_common.h(324): warning C4324: 'RTCPointQueryFunctionArguments': structure was padded due to alignment specifier

We're supposed to ignore warnings in thirdparty code, but this one might be included by some of our own code so it still gets emitted.

The code is still the same upstream in the latest release, and I suppose this alignment/padding is intentional: https://github.com/embree/embree/blob/489b746c0d5010e0da10345e9dc96768bec9a037/include/embree3/rtcore_common.h#L226-L234

I don't get why VS wants to warn about something that the user explicitly asked to do. We might want to ignore this one in modules/raycast.

Edit: Silenced in #66545.

[bruvzg]

Checked warnings in the TextServers as a most familiar part of code:

Warnings from the headers seems to be repeated multiple times, so the real number is much smaller.

• A bunch of C4100, unused function arguments in the virtual / dummy methods, I guess we can ignore it.
• BitField internally use uint32_t but VARIANT_BITFIELD_CAST is using int64_t (probably BitField should be changed to be 64-but like most of the ints and for better compatibility with GDExtension and Variant).
• script_instance is declared in Object and inside wrappers generated by GDVIRTUAL*, should not cause any bugs, but probably should be renamed in the binding generator.
• A bunch of possible loss of data warnings, I'm not sure what to do with them, it's pretty much inevitable when int64_t is used, and adding casts to every case seems like an overkill to me.

[akien-mga]

C4389: 'equality-operator' : signed/unsigned mismatch

2 occurrences.

modules\gridmap\grid_map.cpp(1152): warning C4389: '==': signed/unsigned mismatch
scene\resources\texture.cpp(3108): warning C4389: '!=': signed/unsigned mismatch

Typical int / uint comparisons, since it's only two occurrences for this specific warning it should be easy to fix by changing the type for one of the operands, or cast it.

Edit: Fixed in #66545.

[akien-mga]

C4456: declaration of 'identifier' hides previous local declaration

3 occurrences, in thirdparty code.

C:\Users\Remi\Documents\Godot\godot.git\thirdparty\minimp3\minimp3_ex.h(380): warning C4456: declaration of 'ret' hides previous local declaration
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\minimp3\minimp3_ex.h(565): warning C4456: declaration of 'readed' hides previous local declaration
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\minimp3\minimp3_ex.h(603): warning C4456: declaration of 'readed' hides previous local declaration

Valid warning, similar to -Wshadow=local, should be fixed upstream.

Edit: Fixed in #66545.

[akien-mga]

C4459: declaration of 'identifier' hides global declaration

1 occurrence:

editor\editor_fonts.cpp(129): warning C4459: declaration of 'default_font_size' hides global declaration

Legit warning, it shadows:

scene/resources/default_theme/default_theme.h
36:const int default_font_size = 16;

Edit: Fixed in #66545.

[akien-mga]

C4701: Potentially uninitialized local variable 'name' used

17 occurrences.

C:\Users\Remi\Documents\Godot\godot.git\core\math\bvh_tree.h(262) : warning C4701: potentially uninitialized local variable 'sibling_id' used
C:\Users\Remi\Documents\Godot\godot.git\core\templates\paged_array.h(330) : warning C4701: potentially uninitialized local variable 'remainder_page_id' used
C:\Users\Remi\Documents\Godot\godot.git\drivers\vulkan\rendering_device_vulkan.cpp(4931) : warning C4701: potentially uninitialized local variable 'info' used
C:\Users\Remi\Documents\Godot\godot.git\drivers\vulkan\rendering_device_vulkan.cpp(5005) : warning C4701: potentially uninitialized local variable 'sconst' used
C:\Users\Remi\Documents\Godot\godot.git\drivers\vulkan\vulkan_context.cpp(658) : warning C4701: potentially uninitialized local variable 'vrsProperties' used
C:\Users\Remi\Documents\Godot\godot.git\drivers\vulkan\vulkan_context.cpp(671) : warning C4701: potentially uninitialized local variable 'multiviewProperties' used
C:\Users\Remi\Documents\Godot\godot.git\platform\uwp\export\app_packager.cpp(337) : warning C4701: potentially uninitialized local variable 'strm' used
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_2d\godot_space_2d.cpp(194) : warning C4701: potentially uninitialized local variable 'res_obj' used
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_2d\godot_space_2d.cpp(201) : warning C4701: potentially uninitialized local variable 'res_shape' used
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_space_3d.cpp(189) : warning C4701: potentially uninitialized local variable 'res_obj' used
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_space_3d.cpp(198) : warning C4701: potentially uninitialized local variable 'res_shape' used
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\shader_language.cpp(5442) : warning C4701: potentially uninitialized local variable 'data_type' used
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\shader_language.cpp(5525) : warning C4701: potentially uninitialized local variable 'ident_type' used
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\shader_language.cpp(6669) : warning C4701: potentially uninitialized local variable 'nv' used

Similar to -Wmaybe-uninitialized, we usually aim to fix or work around this one.

Edit: Fixed in #66544 and #66548.

[akien-mga]

C4702: unreachable code

26 occurrences.

C:\Users\Remi\Documents\Godot\godot.git\core\input\input.cpp(368) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\core\variant\variant.cpp(1105) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\core\variant\variant.cpp(1933) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\core\variant\variant.cpp(3575) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\2d\audio_listener_2d.cpp(106) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\3d\audio_listener_3d.cpp(142) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\gui\base_button.cpp(295) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\gui\rich_text_label.cpp(133) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\gui\rich_text_label.cpp(88) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\resources\animation.cpp(2109) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\resources\font.cpp(134) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\resources\material.cpp(3127) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\resources\tile_set.cpp(1540) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\scene\resources\visual_shader_particle_nodes.cpp(1157) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_2d\godot_space_2d.cpp(1040) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(740) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(744) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(747) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(750) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(752) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(755) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_shape_3d.cpp(760) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\renderer_rd\environment\fog.cpp(126) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(286) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\renderer_rd\storage_rd\particles_storage.cpp(1878) : warning C4702: unreachable code
C:\Users\Remi\Documents\Godot\godot.git\servers\rendering\shader_language.cpp(4400) : warning C4702: unreachable code

Probably legit, worth fixing by removing the dead code or fixing potential bugs that made it unreachable.

Edit: Fixed in #66543 and #66568.

[akien-mga]

C4703: Potentially uninitialized local pointer variable 'name' used

2 occurrences.

C:\Users\Remi\Documents\Godot\godot.git\servers\physics_2d\godot_space_2d.cpp(194) : warning C4703: potentially uninitialized local pointer variable 'res_obj' used
C:\Users\Remi\Documents\Godot\godot.git\servers\physics_3d\godot_space_3d.cpp(189) : warning C4703: potentially uninitialized local pointer variable 'res_obj' used

Similar to C4701 which also points at that one.

Edit: Fixed in #66548.

[akien-mga]

C4706: assignment within conditional expression

37 occurrences.

C:\Users\Remi\Documents\Godot\godot.git\core\string\ustring.cpp(2629) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\core\string\ustring.cpp(2658) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\core\string\ustring.cpp(2678) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\core\string\ustring.cpp(2692) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\core\string\ustring.cpp(2706) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\core\templates\hashfuncs.h(66) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2538) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2540) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2542) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2544) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2546) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2548) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2550) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2552) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2554) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2556) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\editor\plugins\canvas_item_editor_plugin.cpp(2558) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\theora\video_stream_theora.cpp(251) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\theora\video_stream_theora.cpp(266) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(156) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(157) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(293) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(294) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(297) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(344) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(345) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\audio_stream_ogg_vorbis.cpp(349) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(113) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(119) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(121) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(130) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(135) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\modules\vorbis\resource_importer_ogg_vorbis.cpp(145) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\platform\windows\godot_windows.cpp(90) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\scene\gui\file_dialog.cpp(181) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\minimp3\minimp3_ex.h(532) : warning C4706: assignment within conditional expression
C:\Users\Remi\Documents\Godot\godot.git\thirdparty\minimp3\minimp3_ex.h(593) : warning C4706: assignment within conditional expression

Aside from the thirdparty code, this can be worth fixing as it's about something we specifically don't want used in the Godot codebase (even if on some aspects it's technically superior / cleaner, but the readability and possibility to introduce bugs is evaluated as too problematic).

Edit: Fixed in #66542 and #66545.