Static analyzer and warnings

pierre-dejoue · Aug 26, 2023 · 9b619a0 · 9b619a0
1 parent 5ba997e
commit 9b619a0
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 17 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -23,13 +23,14 @@ jobs:
     runs-on: ${{ matrix.os }}-latest
 
     env:
+      CMAKE_CONFIG_PROJECT: -DPICROSS_BUILD_CLI=ON -DPICROSS_BUILD_TESTS=ON -DPICROSS_BUILD_EXAMPLES=ON
       CMAKE_CONFIG_EXTRA: ${{ matrix.os == 'macos' && format('-DCMAKE_OSX_ARCHITECTURES={0}', matrix.arch) || '' }}
 
     steps:
     - uses: actions/checkout@v3
 
     - name: Configure CMake
-      run: cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DPICROSS_BUILD_CLI=ON -DPICROSS_BUILD_TESTS=ON ${{ env.CMAKE_CONFIG_EXTRA }}
+      run: cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ env.CMAKE_CONFIG_PROJECT }} ${{ env.CMAKE_CONFIG_EXTRA }}
 
     - name: Build
       working-directory: ${{ github.workspace }}/build

diff --git a/.github/workflows/packaging.yml b/.github/workflows/packaging.yml
@@ -21,6 +21,7 @@ jobs:
     runs-on: ${{ matrix.os }}-latest
 
     env:
+      CMAKE_CONFIG_PROJECT: -DPICROSS_BUILD_CLI=ON -DPICROSS_BUILD_APP=ON
       CMAKE_CONFIG_EXTRA: ${{ matrix.os == 'macos' && format('-DCMAKE_OSX_ARCHITECTURES={0}', matrix.arch) || '' }}
 
     steps:
@@ -33,7 +34,7 @@ jobs:
         sudo apt-get install -y xorg-dev libgl1-mesa-dev
 
     - name: Configure CMake
-      run: cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DPICROSS_BUILD_CLI=ON -DPICROSS_BUILD_APP=ON ${{ env.CMAKE_CONFIG_EXTRA }}
+      run: cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} ${{ env.CMAKE_CONFIG_PROJECT }} ${{ env.CMAKE_CONFIG_EXTRA }}
 
     - name: Build
       working-directory: ${{ github.workspace }}/build

diff --git a/cmake/config/compiler_options.cmake b/cmake/config/compiler_options.cmake
@@ -30,6 +30,10 @@ function(
     /permissive- # standards conformance mode for MSVC compiler.
   )
 
+  set(MSVC_ANALYZER
+    $<$<CONFIG:DEBUG>:/analyze>   # Visual Studio static analyzer
+  )
+
   set(CLANG_WARNINGS
     -Wall
     -Wextra # reasonable and standard
@@ -61,13 +65,13 @@ function(
 
   if(WARNINGS_AS_ERRORS)
     message(TRACE "Warnings are treated as errors")
+    list(APPEND MSVC_WARNINGS /WX)
     list(APPEND CLANG_WARNINGS -Werror)
     list(APPEND GCC_WARNINGS -Werror)
-    list(APPEND MSVC_WARNINGS /WX)
   endif()
 
   if(MSVC)
-    set(_WARNINGS_CXX ${MSVC_WARNINGS})
+    set(_WARNINGS_CXX ${MSVC_WARNINGS} ${MSVC_ANALYZER})
   elseif(CMAKE_CXX_COMPILER_ID MATCHES ".*Clang")
     set(_WARNINGS_CXX ${CLANG_WARNINGS})
   elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")

diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt
@@ -4,7 +4,6 @@
 
 # Dependencies
 include(argagg)
-
 include(compiler_options)
 
 set(CLI_SOURCES
@@ -23,7 +22,7 @@ target_link_libraries(picross_solver_cli
     argagg-lib
 )
 
-set_target_warnings(picross_solver_cli OFF)
+set_target_warnings(picross_solver_cli ON)
 
 install(TARGETS picross_solver_cli)
 

diff --git a/src/cli/src/argagg_wrap.h b/src/cli/src/argagg_wrap.h
@@ -0,0 +1,12 @@
+#pragma once
+
+#ifdef _MSC_VER
+#pragma warning( push )
+#pragma warning( disable : 28020 )               // Warning C28020: The expression 'expr' is not true at this call
+#endif
+
+#include <argagg/argagg.hpp>
+
+#ifdef _MSC_VER
+#pragma warning( pop )
+#endif
diff --git a/src/cli/src/main.cpp b/src/cli/src/main.cpp
@@ -18,7 +18,7 @@
 #include <utils/input_grid_utils.h>
 #include <utils/picross_file_io.h>
 
-#include <argagg/argagg.hpp>
+#include "argagg_wrap.h"
 
 #include <cassert>
 #include <chrono>

diff --git a/src/examples/CMakeLists.txt b/src/examples/CMakeLists.txt
@@ -15,4 +15,4 @@ target_link_libraries(example_1
     picross::picross
 )
 
-set_target_warnings(example_1 OFF)
+set_target_warnings(example_1 ON)
diff --git a/src/tests/picross/src/bench_line_alternatives.cpp b/src/tests/picross/src/bench_line_alternatives.cpp
@@ -41,7 +41,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("N_100_K_1") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return reduction;
         };
         if (bench_run)
         {
@@ -59,7 +60,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("N_100_K_2") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return  true;
         };
         if (bench_run)
         {
@@ -77,7 +79,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("COL 17") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return reduction;
         };
         if (bench_run)
         {
@@ -95,7 +98,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("ROW 45") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return reduction;
         };
         if (bench_run)
         {
@@ -113,7 +117,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("ROW 46") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return reduction;
         };
         if (bench_run)
         {
@@ -131,7 +136,8 @@ TEST_CASE("Bench full reduction", "[line_alternatives]")
         bool bench_run = false;
         BENCHMARK("ROW 48") {
             reduction = full_reduction(constraint, known_tiles);
-            return bench_run = true;
+            bench_run = true;
+            return reduction;
         };
         if (bench_run)
         {
@@ -159,7 +165,8 @@ TEST_CASE("Bench linear vs full reduction", "[line_alternatives]")
             std::string bench_name = "Threes " + std::to_string(nb_alt) + " linear";
             BENCHMARK(std::move(bench_name)) {
                 reduction = linear_reduction(constraint, known_tiles);
-                return bench_run = true;
+                bench_run = true;
+                return reduction;
             };
             if (bench_run)
             {
@@ -173,7 +180,8 @@ TEST_CASE("Bench linear vs full reduction", "[line_alternatives]")
             std::string bench_name = "Threes " + std::to_string(nb_alt) + " full";
             BENCHMARK(std::move(bench_name)) {
                 reduction = full_reduction(constraint, known_tiles);
-                return bench_run = true;
+                bench_run = true;
+                return reduction;
             };
             if (bench_run)
             {
@@ -187,7 +195,8 @@ TEST_CASE("Bench linear vs full reduction", "[line_alternatives]")
             std::string bench_name = "Threes " + std::to_string(nb_alt) + " full buf";
             BENCHMARK(std::move(bench_name)) {
                 reduction = full_reduction(constraint, known_tiles, &buffers);
-                return bench_run = true;
+                bench_run = true;
+                return reduction;
             };
             if (bench_run)
             {