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

refactor: fix conversion warnings on Clang #245

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

JohelEGP
Copy link
Contributor

Previously:

/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:571:66: warning: implicit conversion from 'const unsigned int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  571 |             io.DisplaySize = ImVec2(event.size.width, event.size.height);
      |                              ~~~~~~                   ~~~~~~~~~~~^~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:571:48: warning: implicit conversion from 'const unsigned int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  571 |             io.DisplaySize = ImVec2(event.size.width, event.size.height);
      |                              ~~~~~~ ~~~~~~~~~~~^~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:574:68: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  574 |             io.AddMousePosEvent(event.mouseMove.x, event.mouseMove.y);
      |                ~~~~~~~~~~~~~~~~                    ~~~~~~~~~~~~~~~~^
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:574:49: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  574 |             io.AddMousePosEvent(event.mouseMove.x, event.mouseMove.y);
      |                ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~^
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:803:45: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
  803 |     if (!texture.create(sf::Vector2u(width, height))) {
      |                         ~~                  ^~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:803:38: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
  803 |     if (!texture.create(sf::Vector2u(width, height))) {
      |                         ~~           ^~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:992:62: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  992 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                                                  ~~~~~~~~~~~~^~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:992:28: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  992 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                ~~~~~~~~~~~~^~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:994:33: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  994 |                (textureRect.top + textureRect.height) / textureSize.y);
      |                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:993:34: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
  993 |     ImVec2 uv1((textureRect.left + textureRect.width) / textureSize.x,
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1057:62: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1057 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                                                  ~~~~~~~~~~~~^~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1057:28: warning: implicit conversion from 'const int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1057 |     ImVec2 uv0(textureRect.left / textureSize.x, textureRect.top / textureSize.y);
      |                ~~~~~~~~~~~~^~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1059:33: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1059 |                (textureRect.top + textureRect.height) / textureSize.y);
      |                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1058:34: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1058 |     ImVec2 uv1((textureRect.left + textureRect.width) / textureSize.x,
      |                 ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~  ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1242:61: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1242 |                 if (clip_rect.x < fb_width && clip_rect.y < fb_height && clip_rect.z >= 0.0f &&
      |                                                           ~ ^~~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1242:35: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1242 |                 if (clip_rect.x < fb_width && clip_rect.y < fb_height && clip_rect.z >= 0.0f &&
      |                                 ~ ^~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1245:55: warning: implicit conversion from 'int' to 'float' may lose precision [-Wimplicit-int-float-conversion]
 1245 |                     glScissor((int)clip_rect.x, (int)(fb_height - clip_rect.w),
      |                                                       ^~~~~~~~~ ~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1275:18: warning: implicit conversion changes signedness: 'GLint' (aka 'int') to 'GLenum' (aka 'unsigned int') [-Wsign-conversion]
 1275 |     glShadeModel(last_shade_model);
      |     ~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
/home/johel/Documents/C++/Forks/SFML/imgui-sfml/imgui-SFML.cpp:1331:89: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
 1331 |             bool isPressed = sf::Joystick::isButtonPressed(s_currWindowCtx->joystickId, i);
      |                              ~~                                                         ^
19 warnings generated.

@oprypin
Copy link
Member

oprypin commented Jul 30, 2023

Could you explain how you're getting these warnings? I could not observe them no matter what I tried, for example:

$ clang++ -Werror -Wall -Wextra -I. -I../sfml/include -I../cimgui/imgui examples/minimal/main.cpp imgui-SFML.cpp -c

$ CC=clang CXX=clang++ cmake -DCMAKE_CXX_FLAGS="-Werror -Wall -Wextra" . -DIMGUI_DIR=../imgui -DSFML_DIR=../sfml && cmake --build .
-- Found SFML 2.6.0 in ../sfml
-- Found ImGui v1.89.8 WIP in ../imgui

@oprypin
Copy link
Member

oprypin commented Jul 30, 2023

$ clang++ --version                                                                                                
clang version 15.0.7
Target: x86_64-pc-linux-gnu

@JohelEGP
Copy link
Contributor Author

It's probably -Wconversion.

@oprypin
Copy link
Member

oprypin commented Jul 30, 2023

Ah yes thanks. Needed both -Wconversion -Wsign-conversion

@oprypin
Copy link
Member

oprypin commented Jul 30, 2023

Hm I think the float conversion ones are totally harmless. Up to 224, float covers int. And it's not like making these places explicit gives us any insights.

Perhaps fixing and enforcing -Wsign-conversion is a good goal, but indeed how to actually enforce them?

@JohelEGP
Copy link
Contributor Author

I'm using Clang 17, and my command lines don't have -Wsign-conversion.
So I'm guessing its effects were integrated into -Wconversion.

@JohelEGP
Copy link
Contributor Author

Perhaps fixing and enforcing -Wsign-conversion is a good goal, but indeed how to actually enforce them?

The way I went about it was doing what others did: https://github.com/search?q=repo%3ASFML%2Fimgui-sfml+warnings&type=pullrequests.

@ChrisThrasher
Copy link
Member

Coincidentally I also have a compiler warning related PR but this time I'm actually adding the warnings to the build script so we don't regress with compiler warnings. See #246.

imgui-SFML.cpp Outdated Show resolved Hide resolved
imgui-SFML.cpp Outdated Show resolved Hide resolved
imgui-SFML.cpp Outdated Show resolved Hide resolved
@ChrisThrasher
Copy link
Member

92dd79c

I started fixing these conversion warnings. MSVC caught a few to begin with.

@oprypin oprypin merged commit 44eacdf into SFML:master Aug 8, 2023
29 checks passed
@JohelEGP JohelEGP deleted the clang_conversion_warnings branch August 8, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants