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

Fixes compilation error on MacOS #2040

Merged
merged 1 commit into from Mar 15, 2022

Conversation

roughbits01
Copy link
Contributor

@roughbits01 roughbits01 commented Mar 14, 2022

  • Has this change been discussed on the forum or in an issue before?
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?
  • If you have additional steps which need to be performed list them as tasks!

Description

This change fixes the following compilation error on MacOS with Clang 13 (mainline clang installed through HomeBrew brew install llvm).

error: implicit conversion from 'size_t' (aka 'unsigned long') to 'CGFloat' (aka 'double') may lose precision [-Werror,-Wimplicit-int-float-conversion]

Tasks

  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

Compile SFML with -Werror,-Wimplicit-int-float-conversion

error: implicit conversion from 'size_t' (aka 'unsigned long') to 'CGFloat' (aka 'double') may lose precision [-Werror,-Wimplicit-int-float-conversion]
@eXpl0it3r eXpl0it3r added this to Backlog in SFML 3.0.0 via automation Mar 14, 2022
@eXpl0it3r eXpl0it3r added this to the 3.0 milestone Mar 14, 2022
@eXpl0it3r eXpl0it3r moved this from Backlog to Clean Up Changes in SFML 3.0.0 Mar 14, 2022
@codecov
Copy link

codecov bot commented Mar 14, 2022

Codecov Report

Merging #2040 (1dcf142) into master (2e6c363) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #2040    +/-   ##
=======================================
  Coverage    7.51%   7.51%            
=======================================
  Files         185     185            
  Lines       15848   15847     -1     
  Branches     4170    4170            
=======================================
  Hits         1191    1191            
- Misses      14297   14524   +227     
+ Partials      360     132   -228     
Impacted Files Coverage Δ
src/SFML/Window/OSX/SFWindowController.mm 0.00% <0.00%> (ø)
src/SFML/System/Err.cpp 0.00% <0.00%> (ø)
src/SFML/Audio/Music.cpp 0.00% <0.00%> (ø)
src/SFML/Audio/Sound.cpp 0.00% <0.00%> (ø)
src/SFML/Network/Ftp.cpp 0.00% <0.00%> (ø)
src/SFML/Network/Http.cpp 0.00% <0.00%> (ø)
src/SFML/Audio/ALCheck.cpp 0.00% <0.00%> (ø)
src/SFML/Graphics/Font.cpp 0.00% <0.00%> (ø)
src/SFML/Graphics/Text.cpp 0.00% <0.00%> (ø)
src/SFML/System/String.cpp 3.36% <0.00%> (ø)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e6c363...1dcf142. Read the comment docs.

@ChrisThrasher
Copy link
Member

Did you also compile the tests and examples with that extra warning you added? If we're going to fix this warning, I'd prefer that we add it to CompilerWarnings.cmake and see what else gets caught when the CI pipeline runs.

Copy link
Member

@ChrisThrasher ChrisThrasher left a comment

Choose a reason for hiding this comment

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

I switched to this same version of Clang and was able to recreate this error without enabling additional compiler warnings so we must already be using this warning. The tests and examples build fine too. Thanks for the contribution!

@eXpl0it3r eXpl0it3r merged commit 79250d9 into SFML:master Mar 15, 2022
SFML 3.0.0 automation moved this from Clean Up Changes to Done Mar 15, 2022
@eXpl0it3r
Copy link
Member

Thanks for this fix! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
SFML 3.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants