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

[akien-mga]

Godot version

4.0.beta (14e1f36e614686f3fea0c74fac3624d1027dd5cc)

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.

[bruvzg]

C4127 - conditional expression is constant

Most are legit places to use constexpr, but some are pointing to the disabled code or questionable, so will need future evaluation.

Disabled code:

• modules\theora\video_stream_theora.cpp(460)

  if (false && pp_inc) {

• scene\animation\animation_player.cpp(410):

  if (false && node_cache->node_2d) {

• servers\rendering\renderer_canvas_cull.cpp(1066):

  if (true || p_width <= 1) {

• servers\rendering\renderer_rd\storage_rd\texture_storage.cpp(1357):

  if (false && RD::get_singleton()->texture_is_format_supported_for_usage(RD::DATA_FORMAT_R8G8B8_UNORM, RD::TEXTURE_USAGE_SAMPLING_BIT | RD::TEXTURE_USAGE_CAN_UPDATE_BIT) && RD::get_singleton()->texture_is_format_supported_for_usage(RD::DATA_FORMAT_R8G8B8_SRGB, RD::TEXTURE_USAGE_SAMPLING_BIT | RD::TEXTURE_USAGE_CAN_UPDATE_BIT)) {

Questionable places (includes non-constant parameter):

• modules\gdscript\gdscript_parser.cpp(3693):

  if (t_hint == PropertyHint::PROPERTY_HINT_FLAGS && p_annotation->resolved_arguments.size() > max_flags) {

• modules\gdscript\gdscript_parser.cpp(3826):

  if ((t_type != Variant::FLOAT || export_type.builtin_type != Variant::INT) && (t_type != Variant::INT || export_type.builtin_type != Variant::FLOAT)) {

• servers\rendering\renderer_rd\forward_clustered\render_forward_clustered.cpp(341):

  if (p_pass_mode == PASS_MODE_DEPTH_MATERIAL || p_pass_mode == PASS_MODE_SDF || ((p_pass_mode == PASS_MODE_SHADOW || p_pass_mode == PASS_MODE_SHADOW_DP) && surf->flags & GeometryInstanceSurfaceDataCache::FLAG_USES_DOUBLE_SIDED_SHADOWS)) {

• servers\rendering\renderer_rd\forward_clustered\render_forward_clustered.cpp(292):

  if ((p_pass_mode == PASS_MODE_COLOR && !(p_color_pass_flags & COLOR_PASS_FLAG_TRANSPARENT)) && !(surf->flags & GeometryInstanceSurfaceDataCache::FLAG_PASS_OPAQUE)) {

• core\os\os.cpp(381):

  if (sizeof(void *) == 4 && p_feature == "32") {

[bruvzg]

C4201 - nonstandard extension used: nameless struct/union

All cases are it the headers, so it's actually only a few warnings:

• Similar code in the Vector*, Color and Quaternion:

  union {
  struct {
      real_t x;
      ...
  };
  real_t coord[x] = { 0 };
  };

• MaterialKey in the scene/resources/canvas_item_material.h
• IndexKey, OctantKey and Cell in the modules/gridmap/grid_map.h

This seems to be relevant only for C89/C90 compatibility and probably can be ignored.

Edit: Silenced by #66595.

[akien-mga]

C4245: conversion from 'type1' to 'type2', signed/unsigned mismatch

59 occurrences after cleaning up duplicates.

May or may not be worth fixing. We fixed a handful of those raised by GCC and Clang, but in far most cases those are harmless and solving them leads to excessive casting.

core\extension\extension_api_dump.cpp(126): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
core\io\file_access_compressed.cpp(286): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_compressed.cpp(287): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_compressed.cpp(288): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_compressed.cpp(305): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access.cpp(462): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_encrypted.cpp(222): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_encrypted.cpp(223): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_memory.cpp(136): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_memory.cpp(137): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_network.cpp(369): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_pack.cpp(316): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_pack.cpp(317): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_zip.cpp(301): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\file_access_zip.cpp(302): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\io\packed_data_container.cpp(268): warning C4245: 'argument': conversion from 'PackedDataContainer::<unnamed-enum-TYPE_DICT>' to 'uint32_t', signed/unsigned mismatch
core\io\packed_data_container.cpp(303): warning C4245: 'argument': conversion from 'PackedDataContainer::<unnamed-enum-TYPE_DICT>' to 'uint32_t', signed/unsigned mismatch
core\io\resource_importer.h(45): warning C4245: 'initializing': conversion from 'ResourceUID::<unnamed-enum-INVALID_ID>' to 'uint64_t', signed/unsigned mismatch
core\io\resource_importer.h(45): warning C4245: 'initializing': conversion from 'ResourceUID::<unnamed-enum-INVALID_ID>' to 'uint64_t', signed/unsigned mismatch
core\io\resource_importer.h(45): warning C4245: 'initializing': conversion from 'ResourceUID::<unnamed-enum-INVALID_ID>' to 'uint64_t', signed/unsigned mismatch
core\math\bvh_pair.inc(40): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
core\math\bvh_structs.inc(104): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
core\math\static_raycaster.h(76): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
core\os\memory.cpp(175): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
core\templates\bin_sorted_array.h(64): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
drivers\gles3\rasterizer_scene_gles3.h(207): warning C4245: 'initializing': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
drivers\vulkan\rendering_device_vulkan.cpp(4146): warning C4245: '=': conversion from 'RenderingDevice::AttachmentFormat::<unnamed-enum-UNUSED_ATTACHMENT>' to 'uint32_t', signed/unsigned mismatch
drivers\windows\file_access_windows.cpp(259): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
drivers\windows\file_access_windows.cpp(260): warning C4245: 'return': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
editor\editor_undo_redo_manager.cpp(323): warning C4245: '=': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
modules\ogg\ogg_packet_sequence.cpp(172): warning C4245: 'initializing': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
modules\ogg\ogg_packet_sequence.cpp(190): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
modules\text_server_adv\text_server_adv.cpp(5039): warning C4245: '=': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
modules\tga\image_loader_tga.cpp(118): warning C4245: '=': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
modules\tga\image_loader_tga.cpp(128): warning C4245: '=': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1137): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1138): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1185): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1186): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1233): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1234): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1259): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\android\export\export_plugin.cpp(1260): warning C4245: 'argument': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
platform\windows\joypad_windows.h(85): warning C4245: '=': conversion from 'int' to 'DWORD', signed/unsigned mismatch
scene\2d\audio_stream_player_2d.h(62): warning C4245: 'initializing': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
scene\3d\audio_stream_player_3d.h(84): warning C4245: 'initializing': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
scene\3d\lightmapper.h(88): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
scene\3d\voxelizer.h(57): warning C4245: '=': conversion from 'Voxelizer::<unnamed-enum-CHILD_EMPTY>' to 'uint32_t', signed/unsigned mismatch
servers\audio\audio_stream.cpp(182): warning C4245: '=': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
servers\audio\audio_stream.h(85): warning C4245: 'initializing': conversion from 'int' to 'unsigned int', signed/unsigned mismatch
servers\physics_3d\godot_soft_body_3d.cpp(1271): warning C4245: 'initializing': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(140): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(142): warning C4245: 'return': conversion from 'int' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(913): warning C4245: 'initializing': conversion from 'RendererSceneRenderRD::ShadowAtlas::<unnamed-enum-QUADRANT_SHIFT>' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(914): warning C4245: 'initializing': conversion from 'RendererSceneRenderRD::ShadowAtlas::<unnamed-enum-QUADRANT_SHIFT>' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\renderer_scene_render_rd.cpp(915): warning C4245: 'initializing': conversion from 'RendererSceneRenderRD::ShadowAtlas::<unnamed-enum-QUADRANT_SHIFT>' to 'uint32_t', signed/unsigned mismatch
servers\rendering\renderer_rd\storage_rd\mesh_storage.cpp(1240): warning C4245: '=': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
servers\rendering\renderer_rd\storage_rd\mesh_storage.cpp(1265): warning C4245: '=': conversion from 'int' to 'uint64_t', signed/unsigned mismatch
servers\rendering\renderer_rd\storage_rd\mesh_storage.h(211): warning C4245: 'initializing': conversion from 'int' to 'uint64_t', signed/unsigned mismatch

Edit: Silenced by #66595.

[akien-mga]

I made PR #66595 to build both the editor and the template with MSVC on CI, using /W4 and with a handful of warnings disabled as discussed above:

# Disable warnings which we don't plan to fix.
# C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism.
# C4201 (non-standard nameless struct/union): Only relevant for C89.
# C4244 C4245 C4267 (narrowing conversions): Unavoidable at this scale.
# C4305 (truncation): double to float or real_t, too hard to avoid.

So we can check the build logs to see what warnings remain and how/whether to fix them.

Build logs: windows-editor-release_debug-W4-20220929.log windows-template-release-W4-20220929.log

If we manage to fix (or ignore) all the remaining ones we should be able to compile with /W4 /WX on CI from now on.

[bruvzg]

Visual Studio 2022 seems to add a lot more warnings (but most of them probably should be ignored as well):

C4514: unreferenced inline function has been removed C4714: function marked as __forceinline not inlined C4820: padding added after construct

I'll add a full log after finishing https://github.com/godotengine/godot/pull/66808

[Zylann]

Maybe related, but even when using the following command:

scons p=windows target=editor warnings=all werror=yes -j4

I get this warning:

core\templates\cowdata.h(135) : error C2220: the following warning is treated as an error
core\templates\cowdata.h(135) : warning C4723: potential divide by 0

There is no division anywhere in cowdata.h, but I'm suspecting it is present in files using the size() function in a division. But why is MSVC not mentionning where the function got force-inlined, I have no idea. It certainly doesn't help, because it could be a valid code issue. Even with verbose=yes, no more details. Quite strange when usually with template errors the trail of instantiations is often very explicit.

I don't get this warning if I just add dev_build=yes.

If I remove werror=yes, the build carries on, but no further occurences of the warning, which raise further my concern that it could be a valid one. Then the build finishes with no other warning.

[Zylann]

Some info: the divide by zero warning occurs specifically in lightmapper_rd.cpp. It consistently occurs when dev_build=no, notably when Godot has already been compiled and one change is introduced in the file.

The only division is at line 295:

    for (int m_i = 0; m_i < mesh_instances.size(); m_i++) {
        if (p_step_function) {
            float p = float(m_i + 1) / mesh_instances.size() * 0.1;

Changing the line like this, gets rid of the warning:

    for (int m_i = 0; m_i < mesh_instances.size(); m_i++) {
        if (p_step_function) {
            float p = float(m_i + 1) / (mesh_instances.size() == 0 ? 1 : mesh_instances.size()) * 0.1;

Even though... I dont see how the compiler deduces a potential division by zero.

[akien-mga]

This is pretty much fixed by all the PRs linked above. We now build with /W4 /WX on CI with some warnings disabled explicitly.

[Zylann]

I still get this error in Godot 4.2:

core\templates\cowdata.h(138) : error C2220: the following warning is treated as an error
core\templates\cowdata.h(138) : warning C4723: potential divide by 0

When compiling on Windows with this command:

scons p=windows target=editor warnings=all werror=yes -j4