From d536b3d2cc7174e6a506170e2c0007ab68338867 Mon Sep 17 00:00:00 2001 From: Chris Thrasher Date: Fri, 7 Jul 2023 14:48:44 -0600 Subject: [PATCH] Use SFML 2.6's set of compiler warnings Some minor tweaks were made to remove Android-specific workarounds that CSFML doesn't need and removing a few warnings that are not feasible to fix at the moment. --- cmake/CompilerWarnings.cmake | 103 +++++++++++++++++++++++++++++ cmake/Config.cmake | 4 ++ cmake/Macros.cmake | 4 ++ include/SFML/Graphics/Font.h | 2 +- include/SFML/Window/Event.h | 2 +- src/SFML/Graphics/Color.cpp | 10 +-- src/SFML/Graphics/Font.cpp | 2 +- src/SFML/Graphics/RenderWindow.cpp | 2 +- src/SFML/Graphics/Shader.cpp | 8 +-- src/SFML/Graphics/Transform.cpp | 2 +- src/SFML/Graphics/VertexBuffer.cpp | 3 +- src/SFML/System/Time.cpp | 8 +-- src/SFML/Window/Window.cpp | 2 +- 13 files changed, 132 insertions(+), 20 deletions(-) create mode 100644 cmake/CompilerWarnings.cmake diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake new file mode 100644 index 0000000..a478329 --- /dev/null +++ b/cmake/CompilerWarnings.cmake @@ -0,0 +1,103 @@ +# from here: +# +# https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md + +# Helper function to enable compiler warnings for a specific set of files +function(set_file_warnings) + option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" TRUE) + + set(MSVC_WARNINGS + /W4 # Baseline reasonable warnings + /w14242 # 'identifier': conversion from 'type1' to 'type1', possible loss of data + /w14254 # 'operator': conversion from 'type1:field_bits' to 'type2:field_bits', possible loss of data + /w14263 # 'function': member function does not override any base class virtual member function + /w14265 # 'classname': class has virtual functions, but destructor is not virtual instances of this class may not be destructed correctly + /w14287 # 'operator': unsigned/negative constant mismatch + /we4289 # nonstandard extension used: 'variable': loop control variable declared in the for-loop is used outside the for-loop scope + /w14296 # 'operator': expression is always 'boolean_value' + /w14311 # 'variable': pointer truncation from 'type1' to 'type2' + /w14545 # expression before comma evaluates to a function which is missing an argument list + /w14546 # function call before comma missing argument list + /w14547 # 'operator': operator before comma has no effect; expected operator with side-effect + /w14549 # 'operator': operator before comma has no effect; did you intend 'operator'? + /w14555 # expression has no effect; expected expression with side- effect + /w14619 # pragma warning: there is no warning number 'number' + /w14640 # Enable warning on thread un-safe static member initialization + /w14826 # Conversion from 'type1' to 'type_2' is sign-extended. This may cause unexpected runtime behavior. + /w14905 # wide string literal cast to 'LPSTR' + /w14906 # string literal cast to 'LPWSTR' + /w14928 # illegal copy-initialization; more than one user-defined conversion has been implicitly applied + # /permissive- # standards conformance mode for MSVC compiler. Disabled until all out-of-the-box Windows SDKs can successfully build with it. + + # Disables, remove when appropriate + /wd4996 # disable warnings about deprecated functions + /wd4068 # disable warnings about unknown pragmas (e.g. #pragma GCC) + /wd4505 # disable warnings about unused functions that might be platform-specific + /wd4800 # disable warnings regarding implicit conversions to bool + ) + + set(CLANG_AND_GCC_WARNINGS + -Wall + -Wextra # reasonable and standard + -Wnon-virtual-dtor # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors + -Wcast-align # warn for potential performance problem casts + -Wunused # warn on anything being unused + -Woverloaded-virtual # warn if you overload (not override) a virtual function + -Wconversion # warn on type conversions that may lose data + -Wsign-conversion # warn on sign conversions + -Wdouble-promotion # warn if float is implicit promoted to double + -Wformat=2 # warn on security issues around functions that format output (ie printf) + # -Wimplicit-fallthrough # warn when a missing break causes control flow to continue at the next case in a switch statement (disabled until better compiler support for explicit fallthrough is available) + -Wnull-dereference # warn if a null dereference is detected + -Wold-style-cast # warn for c-style casts + -Wpedantic # warn if non-standard C++ is used + -Wno-deprecated-declarations # do not warning about deprecated declarations + ) + + # Disable warnings as errors when using Clang on Windows to work around deprecation warnings in Windows APIs + if(SFML_OS_WINDOWS AND (SFML_COMPILER_CLANG OR SFML_COMPILER_CLANG_CL)) + set(WARNINGS_AS_ERRORS FALSE) + endif() + + if(WARNINGS_AS_ERRORS) + set(CLANG_AND_GCC_WARNINGS ${CLANG_AND_GCC_WARNINGS} -Werror) + set(MSVC_WARNINGS ${MSVC_WARNINGS} /WX) + endif() + + set(CLANG_WARNINGS + ${CLANG_AND_GCC_WARNINGS} + -Wno-unknown-warning-option # do not warn on GCC-specific warning diagnostic pragmas + -Wno-c++11-long-long # do not warn on uses of long long + ) + + set(GCC_WARNINGS + ${CLANG_AND_GCC_WARNINGS} + -Wmisleading-indentation # warn if indentation implies blocks where blocks do not exist + -Wduplicated-cond # warn if if / else chain has duplicated conditions + -Wlogical-op # warn about logical operations being used where bitwise were probably wanted + # -Wuseless-cast # warn if you perform a cast to the same type (disabled because it is not portable as some typedefs might vary between platforms) + ) + + # Don't enable -Wduplicated-branches for GCC < 8.1 since it will lead to false positives + # https://github.com/gcc-mirror/gcc/commit/6bebae75035889a4844eb4d32a695bebf412bcd7 + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 8.1) + set(GCC_WARNINGS + ${GCC_WARNINGS} + -Wduplicated-branches # warn if if / else branches have duplicated code + ) + endif() + + if(MSVC) + set(FILE_WARNINGS ${MSVC_WARNINGS}) + elseif(CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") + set(FILE_WARNINGS ${CLANG_WARNINGS}) + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set(FILE_WARNINGS ${GCC_WARNINGS}) + else() + message(AUTHOR_WARNING "No compiler warnings set for '${CMAKE_CXX_COMPILER_ID}' compiler.") + endif() + + foreach(WARNING ${FILE_WARNINGS}) + set_property(SOURCE ${ARGV} APPEND_STRING PROPERTY COMPILE_FLAGS " ${WARNING}") + endforeach() +endfunction() diff --git a/cmake/Config.cmake b/cmake/Config.cmake index d48ba46..c2b7236 100644 --- a/cmake/Config.cmake +++ b/cmake/Config.cmake @@ -46,6 +46,10 @@ endif() # - GNUCXX can still be set on macOS when using Clang if(MSVC) set(SFML_COMPILER_MSVC 1) + + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + set(SFML_COMPILER_CLANG_CL 1) + endif() elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(SFML_COMPILER_CLANG 1) diff --git a/cmake/Macros.cmake b/cmake/Macros.cmake index 9d86aec..1d92d09 100644 --- a/cmake/Macros.cmake +++ b/cmake/Macros.cmake @@ -1,4 +1,5 @@ include(CMakeParseArguments) +include(${PROJECT_SOURCE_DIR}/cmake/CompilerWarnings.cmake) # add a new target which is a CSFML library # ex: csfml_add_library(csfml-graphics @@ -15,6 +16,9 @@ macro(csfml_add_library target) # add the CSFML header path target_include_directories(${target} PUBLIC ${PROJECT_SOURCE_DIR}/include) + # add warnings + set_file_warnings(${THIS_SOURCES}) + # define the export symbol of the module string(REPLACE "-" "_" NAME_UPPER "${target}") string(TOUPPER "${NAME_UPPER}" NAME_UPPER) diff --git a/include/SFML/Graphics/Font.h b/include/SFML/Graphics/Font.h index 84455a8..cd2551b 100644 --- a/include/SFML/Graphics/Font.h +++ b/include/SFML/Graphics/Font.h @@ -201,7 +201,7 @@ CSFML_GRAPHICS_API void sfFont_setSmooth(sfFont* font, sfBool smooth); /// \see setSmooth /// //////////////////////////////////////////////////////////// -CSFML_GRAPHICS_API const sfBool sfFont_isSmooth(const sfFont* font); +CSFML_GRAPHICS_API sfBool sfFont_isSmooth(const sfFont* font); //////////////////////////////////////////////////////////// /// \brief Get the font information diff --git a/include/SFML/Window/Event.h b/include/SFML/Window/Event.h index cb0e26f..b6d1ad4 100644 --- a/include/SFML/Window/Event.h +++ b/include/SFML/Window/Event.h @@ -65,7 +65,7 @@ typedef enum sfEvtTouchEnded, ///< A touch event ended (data in event.touch) sfEvtSensorChanged, ///< A sensor value changed (data in event.sensor) - sfEvtCount, ///< Keep last -- the total number of event types + sfEvtCount ///< Keep last -- the total number of event types } sfEventType; diff --git a/src/SFML/Graphics/Color.cpp b/src/SFML/Graphics/Color.cpp index 3280a84..01f8d7b 100644 --- a/src/SFML/Graphics/Color.cpp +++ b/src/SFML/Graphics/Color.cpp @@ -66,10 +66,10 @@ sfColor sfColor_fromRGBA(sfUint8 red, sfUint8 green, sfUint8 blue, sfUint8 alpha //////////////////////////////////////////////////////////// sfColor sfColor_fromInteger(sfUint32 color) { - sfUint8 red = (color & 0xff000000) >> 24; - sfUint8 green = (color & 0x00ff0000) >> 16; - sfUint8 blue = (color & 0x0000ff00) >> 8; - sfUint8 alpha = (color & 0x000000ff) >> 0; + sfUint8 red = static_cast((color & 0xff000000) >> 24); + sfUint8 green = static_cast((color & 0x00ff0000) >> 16); + sfUint8 blue = static_cast((color & 0x0000ff00) >> 8); + sfUint8 alpha = static_cast((color & 0x000000ff) >> 0); return sfColor_fromRGBA(red, green, blue, alpha); } @@ -78,7 +78,7 @@ sfColor sfColor_fromInteger(sfUint32 color) //////////////////////////////////////////////////////////// sfUint32 sfColor_toInteger(sfColor color) { - return (color.r << 24) | (color.g << 16) | (color.b << 8) | color.a; + return static_cast((color.r << 24) | (color.g << 16) | (color.b << 8) | color.a); } diff --git a/src/SFML/Graphics/Font.cpp b/src/SFML/Graphics/Font.cpp index c647604..adac83a 100644 --- a/src/SFML/Graphics/Font.cpp +++ b/src/SFML/Graphics/Font.cpp @@ -168,7 +168,7 @@ void sfFont_setSmooth(sfFont* font, sfBool smooth) //////////////////////////////////////////////////////////// -const sfBool sfFont_isSmooth(const sfFont *font) +sfBool sfFont_isSmooth(const sfFont *font) { if (font->This.isSmooth()) { diff --git a/src/SFML/Graphics/RenderWindow.cpp b/src/SFML/Graphics/RenderWindow.cpp index 0b2d361..9dc05b0 100644 --- a/src/SFML/Graphics/RenderWindow.cpp +++ b/src/SFML/Graphics/RenderWindow.cpp @@ -347,7 +347,7 @@ sfWindowHandle sfRenderWindow_getSystemHandle(const sfRenderWindow* renderWindow { CSFML_CHECK_RETURN(renderWindow, 0); - return (sfWindowHandle)renderWindow->This.getSystemHandle(); + return static_cast(renderWindow->This.getSystemHandle()); } diff --git a/src/SFML/Graphics/Shader.cpp b/src/SFML/Graphics/Shader.cpp index 351a94a..dacc5a4 100644 --- a/src/SFML/Graphics/Shader.cpp +++ b/src/SFML/Graphics/Shader.cpp @@ -269,10 +269,10 @@ void sfShader_setIvec4Uniform(sfShader* shader, const char* name, sfGlslIvec4 ve void sfShader_setIntColorUniform(sfShader* shader, const char* name, sfColor color) { sfGlslIvec4 ivec4; - ivec4.x = (int)color.r; - ivec4.y = (int)color.g; - ivec4.z = (int)color.b; - ivec4.w = (int)color.a; + ivec4.x = static_cast(color.r); + ivec4.y = static_cast(color.g); + ivec4.z = static_cast(color.b); + ivec4.w = static_cast(color.a); sfShader_setIvec4Uniform(shader, name, ivec4); } diff --git a/src/SFML/Graphics/Transform.cpp b/src/SFML/Graphics/Transform.cpp index 8aa4838..c4b45ae 100644 --- a/src/SFML/Graphics/Transform.cpp +++ b/src/SFML/Graphics/Transform.cpp @@ -165,7 +165,7 @@ sfBool sfTransform_equal(sfTransform* left, sfTransform* right) CSFML_CHECK_RETURN(right, false); for (int i = 0; i < 9; ++i) { - if (left->matrix[i] != left->matrix[i]) return sfFalse; + if (left->matrix[i] != right->matrix[i]) return sfFalse; } return sfTrue; } diff --git a/src/SFML/Graphics/VertexBuffer.cpp b/src/SFML/Graphics/VertexBuffer.cpp index 6374b4b..05e98f2 100644 --- a/src/SFML/Graphics/VertexBuffer.cpp +++ b/src/SFML/Graphics/VertexBuffer.cpp @@ -69,7 +69,8 @@ void sfVertexBuffer_destroy(sfVertexBuffer* vertexBuffer) //////////////////////////////////////////////////////////// unsigned int sfVertexBuffer_getVertexCount(const sfVertexBuffer* vertexBuffer) { - CSFML_CALL_RETURN(vertexBuffer, getVertexCount(), 0); + CSFML_CHECK_RETURN(vertexBuffer, 0); + return static_cast(vertexBuffer->This.getVertexCount()); } diff --git a/src/SFML/System/Time.cpp b/src/SFML/System/Time.cpp index b1efb75..747f1d7 100644 --- a/src/SFML/System/Time.cpp +++ b/src/SFML/System/Time.cpp @@ -35,14 +35,14 @@ sfTime sfTime_Zero = {0}; //////////////////////////////////////////////////////////// float sfTime_asSeconds(sfTime time) { - return time.microseconds / 1000000.f; + return static_cast(time.microseconds) / 1000000.f; } //////////////////////////////////////////////////////////// sfInt32 sfTime_asMilliseconds(sfTime time) { - return static_cast(time.microseconds / 1000); + return static_cast(time.microseconds / 1000); } @@ -57,7 +57,7 @@ sfInt64 sfTime_asMicroseconds(sfTime time) sfTime sfSeconds(float amount) { sfTime time; - time.microseconds = static_cast(amount * 1000000); + time.microseconds = static_cast(amount * 1000000); return time; } @@ -66,7 +66,7 @@ sfTime sfSeconds(float amount) sfTime sfMilliseconds(sfInt32 amount) { sfTime time; - time.microseconds = static_cast(amount) * 1000; + time.microseconds = static_cast(amount) * 1000; return time; } diff --git a/src/SFML/Window/Window.cpp b/src/SFML/Window/Window.cpp index 5913ea3..6079f62 100644 --- a/src/SFML/Window/Window.cpp +++ b/src/SFML/Window/Window.cpp @@ -320,5 +320,5 @@ sfWindowHandle sfWindow_getSystemHandle(const sfWindow* window) { CSFML_CHECK_RETURN(window, 0); - return (sfWindowHandle)window->This.getSystemHandle(); + return static_cast(window->This.getSystemHandle()); }