Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings emitted with -Wall #1478

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

richardhozak
Copy link
Contributor

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.

@richardhozak richardhozak requested a review from a team as a code owner June 2, 2024 19:55
@AThousandShips
Copy link
Member

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

@Calinou Calinou added the bug This has been identified as a bug label Jun 3, 2024
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! These changes look good to me.

@dsnopek dsnopek merged commit 7d4758e into godotengine:master Jun 12, 2024
12 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Jun 14, 2024

Cherry-picked for 4.1 in PR #1491

@dsnopek
Copy link
Collaborator

dsnopek commented Jun 14, 2024

Cherry-picked for 4.2 in PR #1492

@AThousandShips AThousandShips added this to the 4.3 milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants