Skip to content

Commit fabd962

Browse files
skraxskrax
authored
Stricter warnings and linting (#65)
* add stricter warnings to ProjectOptions * undefine warnings produced by ghc_filesystem * turn of -Werr as default * silence warnings produced by cxxopts * add static analyzers add sanitzers * add different configure options for cmake * comment out matrix configure-options * add values to cmake flags * add clang-tidy to install script * silence ghc_fileystem warnings * add newlines to github action * enalbe sanitizers and clang-tidy * disable warnings as errors
1 parent 6d5c0c2 commit fabd962

File tree

10 files changed

+270
-33
lines changed

10 files changed

+270
-33
lines changed

‎.github/workflows/sourcehold.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,14 @@ jobs:
2121
include:
2222
- os: ubuntu-latest
2323
install-dependencies: "sh ./ubuntu/install-dependencies.sh"
24-
cmake-configure-options: ""
24+
#TODO: (seidl, @github: skrax)
25+
# enable warnings as errors later
26+
cmake-configure-options: "-DWARNINGS_AS_ERRORS=OFF
27+
-DENABLE_COVERAGE=ON
28+
-DENABLE_SANITIZER_UNDEFINED_BEHAVIOR=ON
29+
-DENABLE_CLANG_TIDY=ON
30+
-DENABLE_SANITIZER_ADDRESS=ON
31+
-DENABLE_SANITIZER_LEAK=ON"
2532
cmake-build-options: ""
2633
uploaded-artifact-name: "sourcehold-linux-amd64"
2734
- os: macos-latest

‎cmake/Preamble.cmake

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_LIST_DIR})
33
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
44

5+
if(MSVC)
6+
foreach( OUTPUTCONFIG ${CMAKE_CONFIGURATION_TYPES} )
7+
string( TOUPPER ${OUTPUTCONFIG} OUTPUTCONFIG )
8+
set( CMAKE_RUNTIME_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
9+
set( CMAKE_LIBRARY_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
10+
set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
11+
endforeach( OUTPUTCONFIG CMAKE_CONFIGURATION_TYPES )
12+
endif()
13+
14+
515
include(cmake/PreventInSourceBuild.cmake)
616
include(cmake/PlatformCheck.cmake)
717
include(cmake/AddImportedTarget.cmake)

‎cmake/ProjectOptions.cmake

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,9 @@
1+
include(cmake/Warnings.cmake)
2+
include(cmake/StaticAnalyzers.cmake)
3+
include(cmake/Sanitizers.cmake)
4+
15
add_library(ProjectOptions INTERFACE)
26
target_compile_features(ProjectOptions INTERFACE cxx_std_17)
37

4-
if(MSVC)
5-
# TODO(seidl):
6-
# move this somewhere else
7-
target_compile_options(ProjectOptions INTERFACE -D_CRT_SECURE_NO_WARNINGS)
8-
foreach( OUTPUTCONFIG ${CMAKE_CONFIGURATION_TYPES} )
9-
string( TOUPPER ${OUTPUTCONFIG} OUTPUTCONFIG )
10-
set( CMAKE_RUNTIME_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
11-
set( CMAKE_LIBRARY_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
12-
set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${OUTPUTCONFIG} ${CMAKE_BINARY_DIR} )
13-
endforeach( OUTPUTCONFIG CMAKE_CONFIGURATION_TYPES )
14-
else()
15-
if(NOT DEFINED CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "Release")
16-
set(OPTIMIZATION_LEVEL -Ofast)
17-
else()
18-
set(OPTIMIZATION_LEVEL -O0)
19-
endif() # If Release build type
20-
21-
target_compile_options(ProjectOptions INTERFACE
22-
-Wno-reorder
23-
-pedantic-errors
24-
${OPTIMIZATION_LEVEL}
25-
-fno-fast-math)
26-
endif()
27-
# TODO(seidl):
28-
#add stricter warnings and linting
8+
set_project_warnings(ProjectOptions)
9+
enable_sanitizers(ProjectOptions)

‎cmake/Sanitizers.cmake

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
function(enable_sanitizers project_name)
2+
3+
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES ".*Clang")
4+
option(ENABLE_COVERAGE "Enable coverage reporting for gcc/clang" FALSE)
5+
6+
if(ENABLE_COVERAGE)
7+
target_compile_options(${project_name} INTERFACE --coverage -O0 -g)
8+
target_link_libraries(${project_name} INTERFACE --coverage)
9+
endif()
10+
11+
set(SANITIZERS "")
12+
13+
option(ENABLE_SANITIZER_ADDRESS "Enable address sanitizer" FALSE)
14+
if(ENABLE_SANITIZER_ADDRESS)
15+
list(APPEND SANITIZERS "address")
16+
endif()
17+
18+
option(ENABLE_SANITIZER_LEAK "Enable leak sanitizer" FALSE)
19+
if(ENABLE_SANITIZER_LEAK)
20+
list(APPEND SANITIZERS "leak")
21+
endif()
22+
23+
option(ENABLE_SANITIZER_UNDEFINED_BEHAVIOR "Enable undefined behavior sanitizer" FALSE)
24+
if(ENABLE_SANITIZER_UNDEFINED_BEHAVIOR)
25+
list(APPEND SANITIZERS "undefined")
26+
endif()
27+
28+
option(ENABLE_SANITIZER_THREAD "Enable thread sanitizer" FALSE)
29+
if(ENABLE_SANITIZER_THREAD)
30+
if("address" IN_LIST SANITIZERS OR "leak" IN_LIST SANITIZERS)
31+
message(WARNING "Thread sanitizer does not work with Address and Leak sanitizer enabled")
32+
else()
33+
list(APPEND SANITIZERS "thread")
34+
endif()
35+
endif()
36+
37+
option(ENABLE_SANITIZER_MEMORY "Enable memory sanitizer" FALSE)
38+
if(ENABLE_SANITIZER_MEMORY AND CMAKE_CXX_COMPILER_ID MATCHES ".*Clang")
39+
if("address" IN_LIST SANITIZERS
40+
OR "thread" IN_LIST SANITIZERS
41+
OR "leak" IN_LIST SANITIZERS)
42+
message(WARNING "Memory sanitizer does not work with Address, Thread and Leak sanitizer enabled")
43+
else()
44+
list(APPEND SANITIZERS "memory")
45+
endif()
46+
endif()
47+
48+
list(
49+
JOIN
50+
SANITIZERS
51+
","
52+
LIST_OF_SANITIZERS)
53+
54+
endif()
55+
56+
if(LIST_OF_SANITIZERS)
57+
if(NOT
58+
"${LIST_OF_SANITIZERS}"
59+
STREQUAL
60+
"")
61+
target_compile_options(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS})
62+
target_link_libraries(${project_name} INTERFACE -fsanitize=${LIST_OF_SANITIZERS})
63+
endif()
64+
endif()
65+
66+
endfunction()

‎cmake/StaticAnalyzers.cmake

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
option(ENABLE_CPPCHECK "Enable static analysis with cppcheck" OFF)
2+
option(ENABLE_CLANG_TIDY "Enable static analysis with clang-tidy" OFF)
3+
option(ENABLE_INCLUDE_WHAT_YOU_USE "Enable static analysis with include-what-you-use" OFF)
4+
5+
if(ENABLE_CPPCHECK)
6+
find_program(CPPCHECK cppcheck)
7+
if(CPPCHECK)
8+
set(CMAKE_CXX_CPPCHECK
9+
${CPPCHECK}
10+
--suppress=missingInclude
11+
--enable=all
12+
--inline-suppr
13+
--inconclusive
14+
-i
15+
${CMAKE_SOURCE_DIR}/imgui/lib)
16+
else()
17+
message(SEND_ERROR "cppcheck requested but executable not found")
18+
endif()
19+
endif()
20+
21+
if(ENABLE_CLANG_TIDY)
22+
find_program(CLANGTIDY clang-tidy)
23+
if(CLANGTIDY)
24+
set(CMAKE_CXX_CLANG_TIDY ${CLANGTIDY} -extra-arg=-Wno-unknown-warning-option)
25+
else()
26+
message(SEND_ERROR "clang-tidy requested but executable not found")
27+
endif()
28+
endif()
29+
30+
if(ENABLE_INCLUDE_WHAT_YOU_USE)
31+
find_program(INCLUDE_WHAT_YOU_USE include-what-you-use)
32+
if(INCLUDE_WHAT_YOU_USE)
33+
set(CMAKE_CXX_INCLUDE_WHAT_YOU_USE ${INCLUDE_WHAT_YOU_USE})
34+
else()
35+
message(SEND_ERROR "include-what-you-use requested but executable not found")
36+
endif()
37+
endif()

‎cmake/Warnings.cmake

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# from here:
2+
#
3+
# https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md
4+
5+
function(set_project_warnings project_name)
6+
option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" OFF)
7+
8+
set(MSVC_WARNINGS
9+
/W4 # Baseline reasonable warnings
10+
/w14242 # 'identifier': conversion from 'type1' to 'type1', possible loss of data
11+
/w14254 # 'operator': conversion from 'type1:field_bits' to 'type2:field_bits', possible loss of data
12+
/w14263 # 'function': member function does not override any base class virtual member function
13+
/w14265 # 'classname': class has virtual functions, but destructor is not virtual instances of this class may not
14+
# be destructed correctly
15+
/w14287 # 'operator': unsigned/negative constant mismatch
16+
/we4289 # nonstandard extension used: 'variable': loop control variable declared in the for-loop is used outside
17+
# the for-loop scope
18+
/w14296 # 'operator': expression is always 'boolean_value'
19+
/w14311 # 'variable': pointer truncation from 'type1' to 'type2'
20+
/w14545 # expression before comma evaluates to a function which is missing an argument list
21+
/w14546 # function call before comma missing argument list
22+
/w14547 # 'operator': operator before comma has no effect; expected operator with side-effect
23+
/w14549 # 'operator': operator before comma has no effect; did you intend 'operator'?
24+
/w14555 # expression has no effect; expected expression with side- effect
25+
/w14619 # pragma warning: there is no warning number 'number'
26+
/w14640 # Enable warning on thread un-safe static member initialization
27+
/w14826 # Conversion from 'type1' to 'type_2' is sign-extended. This may cause unexpected runtime behavior.
28+
/w14905 # wide string literal cast to 'LPSTR'
29+
/w14906 # string literal cast to 'LPWSTR'
30+
/w14928 # illegal copy-initialization; more than one user-defined conversion has been implicitly applied
31+
/permissive- # standards conformance mode for MSVC compiler.
32+
)
33+
34+
set(CLANG_WARNINGS
35+
-Wall
36+
-Wextra # reasonable and standard
37+
-Wshadow # warn the user if a variable declaration shadows one from a parent context
38+
-Wnon-virtual-dtor # warn the user if a class with virtual functions has a non-virtual destructor. This helps
39+
# catch hard to track down memory errors
40+
-Wold-style-cast # warn for c-style casts
41+
-Wcast-align # warn for potential performance problem casts
42+
-Wunused # warn on anything being unused
43+
-Woverloaded-virtual # warn if you overload (not override) a virtual function
44+
-Wpedantic # warn if non-standard C++ is used
45+
-Wconversion # warn on type conversions that may lose data
46+
-Wsign-conversion # warn on sign conversions
47+
-Wnull-dereference # warn if a null dereference is detected
48+
-Wdouble-promotion # warn if float is implicit promoted to double
49+
-Wformat=2 # warn on security issues around functions that format output (ie printf)
50+
)
51+
52+
if(WARNINGS_AS_ERRORS)
53+
set(CLANG_WARNINGS ${CLANG_WARNINGS} -Werror)
54+
set(MSVC_WARNINGS ${MSVC_WARNINGS} /WX)
55+
endif()
56+
57+
set(GCC_WARNINGS
58+
${CLANG_WARNINGS}
59+
-Wmisleading-indentation # warn if indentation implies blocks where blocks do not exist
60+
-Wduplicated-cond # warn if if / else chain has duplicated conditions
61+
-Wduplicated-branches # warn if if / else branches have duplicated code
62+
-Wlogical-op # warn about logical operations being used where bitwise were probably wanted
63+
-Wuseless-cast # warn if you perform a cast to the same type
64+
)
65+
66+
if(MSVC)
67+
set(PROJECT_WARNINGS ${MSVC_WARNINGS})
68+
elseif(CMAKE_CXX_COMPILER_ID MATCHES ".*Clang")
69+
set(PROJECT_WARNINGS ${CLANG_WARNINGS})
70+
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
71+
set(PROJECT_WARNINGS ${GCC_WARNINGS})
72+
else()
73+
message(AUTHOR_WARNING "No compiler warnings set for '${CMAKE_CXX_COMPILER_ID}' compiler.")
74+
endif()
75+
76+
target_compile_options(${project_name} INTERFACE ${PROJECT_WARNINGS})
77+
78+
endfunction()

‎src/Game.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include <vector>
55
#include <regex>
66

7-
#include <cxxopts.hpp>
7+
#include "System/Commandline.h"
88

99
#include "World.h"
1010
#include "GameManager.h"

‎src/System/Commandline.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#pragma once
2+
#if defined(__clang__)
3+
#pragma GCC diagnostic push
4+
#pragma GCC diagnostic ignored "-Wsign-conversion"
5+
#endif // __clang__
6+
7+
#if defined(__GNUC__) && !defined(__clang__)
8+
#pragma GCC diagnostic push
9+
#pragma GCC diagnostic ignored "-Wconversion"
10+
#pragma GCC diagnostic push
11+
#pragma GCC diagnostic ignored "-Wsign-conversion"
12+
#endif // GCC
13+
14+
#if defined(_MSC_VER)
15+
#endif // _MSC_VER
16+
17+
#include <cxxopts.hpp>
18+
19+
#if defined(__clang__)
20+
#pragma GCC diagnostic pop
21+
#endif // __clang__
22+
23+
#if defined(__GNUC__) && !defined(__clang__)
24+
#pragma GCC diagnostic pop
25+
#pragma GCC diagnostic pop
26+
#endif // GCC
27+
28+
#if defined(_MSC_VER)
29+
#endif // _MSC_VER

‎src/System/filesystem.h

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,41 @@
11
#pragma once
2+
// Disable warnings caused by ghc_filesystem
3+
#if defined(__clang__)
4+
#if __clang_major__ > 10
5+
#pragma GCC diagnostic push
6+
#pragma GCC diagnostic ignored "-Wrange-loop-construct"
7+
#endif // __clang_major__ > 10
8+
#endif // __clang__
9+
10+
#if defined(__GNUC__) && !defined(__clang__)
11+
#pragma GCC diagnostic push
12+
#pragma GCC diagnostic ignored "-Wuseless-cast"
13+
#pragma GCC diagnostic push
14+
#pragma GCC diagnostic ignored "-Wconversion"
15+
#endif // GCC
16+
17+
#ifdef _MSC_VER
18+
#endif // _MSC_VER
219

3-
/**
4-
* Wrapper file for ghc::filesystem, because screw you, whoever
5-
* decided defining macros with generic names like this in a library
6-
* would be a good idea!
7-
*/
820
#include <ghc/filesystem.hpp>
921

22+
#ifdef __clang__
23+
#if __clang_major__ > 10
24+
#pragma GCC diagnostic pop
25+
#endif // __clang_major__ > 10
26+
#endif // __clang__
27+
28+
#if defined(__GNUC__) && !defined(__clang__)
29+
#pragma GCC diagnostic pop
30+
#pragma GCC diagnostic pop
31+
#endif // GCC
32+
33+
#ifdef _MSC_VER
34+
// TODO(seidl):
35+
// investigate MSVC warnings
36+
#endif //_MSC_VER
37+
38+
// undefine macros from ghc_filesystem
1039
#undef ERROR
1140
#undef min
1241
#undef max

‎ubuntu/install-dependencies.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#!/bin/sh
22

3-
sudo apt install libsdl2-dev libopenal-dev libavcodec-dev libavformat-dev libavutil-dev libavfilter-dev libswscale-dev libpostproc-dev libswresample-dev
3+
sudo apt install libsdl2-dev libopenal-dev libavcodec-dev libavformat-dev libavutil-dev libavfilter-dev libswscale-dev libpostproc-dev libswresample-dev clang-tidy

0 commit comments

Comments
 (0)