cpp-best-practices / cmake_template

CMake for C++ Best Practices
The Unlicense
1.09k stars 118 forks source link

Warnings on constexpr testing #88

Open pantolin opened 4 months ago

pantolin commented 4 months ago

Hi guys,

maybe someone here can help me.

I don't quite understand the idea of the constexpr tests. My compiler (Apple Clang 15.0.0) resolves the static checks at compile time, and then throws warnings like

warning: Condition 'factorial_constexpr(0)==1' is always true [knownConditionTrueFalse]
STATIC_REQUIRE(factorial_constexpr(0) == 1);

/.../.../constexpr_tests.cpp:7:37: note: Calling function 'factorial_constexpr' returns 1
STATIC_REQUIRE(factorial_constexpr(0) == 1);

/.../.../constexpr_tests.cpp:7:41: note: Condition 'factorial_constexpr(0)==1' is always true
 STATIC_REQUIRE(factorial_constexpr(0) == 1);

and then, because warnings are promoted to errors, the build fails.

Is this on purpose? I mean, the idea is to check that this is the behavior and then disable the tests?

Thanks.

Pablo

ByunghoonKim commented 1 month ago

Hi, I was also curious about this behavior.

First, the simplest approach to solve this issue would be ignoring the result of [knownConditionTrueFalse] check. Therefore, adding --suppress=knownConditionTrueFalse to somewhere around line 20 of cmake/StaticAnalyzers.cmake would suffice. Like so:

      set(CMAKE_CXX_CPPCHECK
          ${CPPCHECK}
          --template=${CPPCHECK_TEMPLATE}
          --enable=style,performance,warning,portability
          --suppress=knownConditionTrueFalse    # newly added line

Or we can overreact and remove the whole class of checks which seems to include [knownConditionTrueFalse]. Like so:

      set(CMAKE_CXX_CPPCHECK
          ${CPPCHECK}
          --template=${CPPCHECK_TEMPLATE}
          --enable=performance,warning,portability    # removed `style`

This, however, is merely a cure for the symptom. The root cause of this behavior seems to stem from how cppcheck really works. The manual of cppcheck reads

Cppcheck’s preprocessor basically handles includes like any other preprocessor. However, while other preprocessors stop working when they encounter a missing header, Cppcheck will just print an information message and continues parsing the code. The purpose of this behaviour is that Cppcheck is meant to work without necessarily seeing the entire code. Actually, it is recommended to not give all include paths.

This means, when test/constexpr_tests.cpp is processed by cppcheck, it encounters

#include <catch2/catch_test_macros.hpp>

, ignores this line, and moves on. Later it encounters

  STATIC_REQUIRE(factorial_constexpr(0) == 1);

, and without expanding the macro properly cppcheck somehow tries to check this line against its internal rule sets, which results in the behavior you experienced. Therefore, to resolve this issue at this fundamental level, we need to enforce cppcheck to include a header file so that it can expand STATIC_REQUIRE properly. Like so:

      set(CMAKE_CXX_CPPCHECK
          ${CPPCHECK}
          --template=${CPPCHECK_TEMPLATE}
          --enable=style,performance,warning,portability
          --include="${CMAKE_CURRENT_BINARY_DIR}/_deps/catch2-src/src/"    # newly added line

I believe this should solve your problem at the expense of longer compile time.