Enable and fix compiler warnings

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()

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)
 

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)

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

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;
 
 

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<sfUint8>((color & 0xff000000) >> 24);
+    sfUint8 green = static_cast<sfUint8>((color & 0x00ff0000) >> 16);
+    sfUint8 blue =  static_cast<sfUint8>((color & 0x0000ff00) >> 8);
+    sfUint8 alpha = static_cast<sfUint8>((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<sfUint32>((color.r << 24) | (color.g << 16) | (color.b << 8) | color.a);
 }
 
 

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())
     {

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<sfWindowHandle>(renderWindow->This.getSystemHandle());
 }
 
 

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<int>(color.r);
+	ivec4.y = static_cast<int>(color.g);
+	ivec4.z = static_cast<int>(color.b);
+	ivec4.w = static_cast<int>(color.a);
 
 	sfShader_setIvec4Uniform(shader, name, ivec4);
 }

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;
 }

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<unsigned int>(vertexBuffer->This.getVertexCount());
 }
 
 

src/SFML/Network/IpAddress.cpp

@@ -35,8 +35,8 @@ namespace
     // Helper function for converting a SFML address to a CSFML one
     sfIpAddress fromSFMLAddress(sf::IpAddress address)
     {
-        sfIpAddress result;
-        strncpy(result.address, address.toString().c_str(), 16);
+        sfIpAddress result = {0};
+        strncpy(result.address, address.toString().c_str(), 15);
 
         return result;
     }

src/SFML/Network/TcpSocket.cpp

@@ -75,7 +75,7 @@ sfIpAddress sfTcpSocket_getRemoteAddress(const sfTcpSocket* socket)
     CSFML_CHECK_RETURN(socket, result);
 
     sf::IpAddress address = socket->This.getRemoteAddress();
-    strncpy(result.address, address.toString().c_str(), 16);
+    strncpy(result.address, address.toString().c_str(), 15);
 
     return result;
 }

src/SFML/Network/UdpSocket.cpp

@@ -113,7 +113,10 @@ sfSocketStatus sfUdpSocket_receive(sfUdpSocket* socket, void* data, size_t size,
         *received = sizeReceived;
 
     if (remoteAddress)
-        strncpy(remoteAddress->address, address.toString().c_str(), 16);
+    {
+        *remoteAddress = sfIpAddress_None;
+        strncpy(remoteAddress->address, address.toString().c_str(), 15);
+    }
 
     if (remotePort)
         *remotePort = port;
@@ -149,7 +152,10 @@ sfSocketStatus sfUdpSocket_receivePacket(sfUdpSocket* socket, sfPacket* packet,
         return static_cast<sfSocketStatus>(status);
 
     if (remoteAddress)
-        strncpy(remoteAddress->address, address.toString().c_str(), 16);
+    {
+        *remoteAddress = sfIpAddress_None;
+        strncpy(remoteAddress->address, address.toString().c_str(), 15);
+    }
 
     if (remotePort)
         *remotePort = port;

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<float>(time.microseconds) / 1000000.f;
 }
 
 
 ////////////////////////////////////////////////////////////
 sfInt32 sfTime_asMilliseconds(sfTime time)
 {
-    return static_cast<sfUint32>(time.microseconds / 1000);
+    return static_cast<sfInt32>(time.microseconds / 1000);
 }
 
 
@@ -57,7 +57,7 @@ sfInt64 sfTime_asMicroseconds(sfTime time)
 sfTime sfSeconds(float amount)
 {
     sfTime time;
-    time.microseconds = static_cast<sfUint64>(amount * 1000000);
+    time.microseconds = static_cast<sfInt64>(amount * 1000000);
     return time;
 }
 
@@ -66,7 +66,7 @@ sfTime sfSeconds(float amount)
 sfTime sfMilliseconds(sfInt32 amount)
 {
     sfTime time;
-    time.microseconds = static_cast<sfUint64>(amount) * 1000;
+    time.microseconds = static_cast<sfInt64>(amount) * 1000;
     return time;
 }
 

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<sfWindowHandle>(window->This.getSystemHandle());
 }