Skip to content

Commit

Permalink
ci: Enable standard library assertions in debug builds (#3759)
Browse files Browse the repository at this point in the history
Should add assertions that check preconditions of calls to standardlibrary functions.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

- **Chores**
	- Enhanced configuration for compiler flags to conditionally enable assertions.
	- Addressed compilation issues with GCC 13 by undefining specific preprocessor directives.
	- Maintained existing logic for compiler type checks without altering public entity signatures.
- **Bug Fixes**
	- Adjusted the logic for generating random directions to ensure correct bounds for angles.
- **Documentation**
	- Improved comments for clarity on the calculations in the random direction generation function.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
benjaminhuth authored Dec 11, 2024
1 parent 93e3b27 commit 595f2e6
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Core/src/Visualization/ObjVisualization3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void ObjVisualization3D::faces(const std::vector<Vector3>& vtxs,
o.vertices.insert(o.vertices.end(), vtxs.begin(), vtxs.end());
for (const auto& face : faces) {
if (face.size() == 2) {
o.lines.push_back({face[0] + vtxoffs, face[2] + vtxoffs});
o.lines.push_back({face[0] + vtxoffs, face[1] + vtxoffs});
} else {
FaceType rawFace;
std::ranges::transform(
Expand Down
7 changes: 7 additions & 0 deletions Plugins/Json/src/DetrayJsonHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

// For whatever reason, this compilation unit does not compile
// with those assertions and GCC 13. For now just disable the
// flags in this case.
#if defined(_GLIBCXX_ASSERTIONS) && __GNUC__ == 13
#undef _GLIBCXX_ASSERTIONS
#endif

#include "Acts/Plugins/Json/DetrayJsonHelper.hpp"

namespace Acts::DetrayJsonHelper {
Expand Down
11 changes: 3 additions & 8 deletions Tests/UnitTests/Core/Utilities/AlgebraHelpersTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,15 @@ BOOST_AUTO_TEST_CASE(SafeInverseFPESmallMatrix) {
m(1, 1) = 1;

auto mInv = Acts::safeInverse(m);
BOOST_REQUIRE(mInv.has_value());
auto mInvInv = Acts::safeInverse(*mInv);

BOOST_CHECK(mInv);
BOOST_CHECK(!mInvInv);

ACTS_VERBOSE("Test: SafeInverseFPESmallMatrix" << "\n"
<< "m:\n"
<< m << "\n"
<< "mInv:\n"
<< *mInv << "\n"
<< "mInvInv [garbage]:\n"
<< *mInvInv);
<< *mInv);
}

BOOST_AUTO_TEST_CASE(SafeInverseFPELargeMatrix) {
Expand All @@ -107,9 +104,7 @@ BOOST_AUTO_TEST_CASE(SafeInverseFPELargeMatrix) {

ACTS_VERBOSE("Test: SafeInverseFPELargeMatrix" << "\n"
<< "m:\n"
<< m << "\n"
<< "mInv [garbage]:\n"
<< *mInv);
<< m);
}

/// This test should not compile
Expand Down
5 changes: 5 additions & 0 deletions cmake/ActsCompilerOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ set(cxx_flags
"-Wall -Wextra -Wpedantic -Wshadow -Wzero-as-null-pointer-constant -Wold-style-cast"
)

# Add assertions to standard libraries
if(ACTS_FORCE_ASSERTIONS)
set(cxx_flags "${cxx_flags} -D_GLIBCXX_ASSERTIONS -D_LIBCPP_DEBUG")
endif()

# This adds some useful conversion checks like float-to-bool, float-to-int, etc.
# However, at the moment this is only added to clang builds, since GCC's -Wfloat-conversion
# is much more aggressive and also triggers on e.g., double-to-float
Expand Down

0 comments on commit 595f2e6

Please sign in to comment.