godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.69k stars 528 forks source link

Fix warnings emitted with -Wall #1478

Closed richardhozak closed 3 months ago

richardhozak commented 3 months ago

Hello, I am tracking 4.2 branch of godot-cpp for my project and compiling my whole project with -Wall. I recently updated godot-cpp to pull all changes from 4.2 branch and there are some new warnings during build that this PR fixes.

The warnings that are fixed by this are:

src/core/object.cpp:78:27: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::vector<godot::Variant>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare]
   78 |         for (int i = 0; i < default_arguments.size(); i++) {
      |                         ~~^~~~~~~~~~~~~~~~~~~~~~~~~~

src/core/class_db.cpp:356:35: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Wsign-compare]
  356 |                 for (int i = 0; i < mi.argument_count; i++) {
      |                                 ~~^~~~~~~~~~~~~~~~~~~

src/core/class_db.cpp:432:69: warning: loop variable ‘pair’ of type ‘const std::pair<godot::StringName, godot::Object*>&’ binds to a temporary constructed from type ‘std::pair<const godot::StringName, godot::Object*>’ [-Wrange-loop-construct]
  432 |                         for (const std::pair<StringName, Object *> &pair : engine_singletons) {
      |                                                                     ^~~~

src/core/class_db.cpp:432:69: note: use non-reference type ‘const std::pair<godot::StringName, godot::Object*>’ to make the copy explicit or ‘const std::pair<const godot::StringName, godot::Object*>&’ to prevent copying

The first two are just changing the int type to match the one on right hand side and the third one is just what gcc suggested.

AThousandShips commented 3 months ago

These are valid changes generally but more generally we don't necessarily support building with every possible warning setting, so any future niche errors that might be fired by a setting we don't have as part of our build setup might not be as appropriate, though again in this case they should be valid

dsnopek commented 3 months ago

Cherry-picked for 4.1 in PR https://github.com/godotengine/godot-cpp/pull/1491

dsnopek commented 3 months ago

Cherry-picked for 4.2 in PR https://github.com/godotengine/godot-cpp/pull/1492