From 99c80c09e2fbee02a28254607074a2e1b64fa7e9 Mon Sep 17 00:00:00 2001 From: Paul Gessinger <hello@paulgessinger.com> Date: Fri, 9 Feb 2018 14:28:33 +0100 Subject: [PATCH] Introduce throw_assert This does a runtime check even when not in debug (unlike plain assert) and throws an exception in case the assertion fails. This is used in a couple of places. I added tests to check the asserts actually throw. --- Core/include/ACTS/Utilities/BinningData.hpp | 6 +- Core/include/ACTS/Utilities/ThrowAssert.hpp | 90 +++++++++++++++++++++ Core/src/Surfaces/ConeSurface.cpp | 3 +- Core/src/Surfaces/CylinderSurface.cpp | 3 +- Core/src/Surfaces/DiamondBounds.cpp | 5 +- Core/src/Surfaces/DiscSurface.cpp | 3 +- Core/src/Surfaces/LineSurface.cpp | 4 +- Core/src/Surfaces/PlaneSurface.cpp | 3 +- Tests/Surfaces/ConeSurfaceTests.cpp | 4 + Tests/Surfaces/CylinderSurfaceTests.cpp | 4 + Tests/Surfaces/DetectorElementStub.hpp | 2 + Tests/Surfaces/DiamondBoundsTests.cpp | 8 ++ Tests/Surfaces/DiscSurfaceTests.cpp | 6 ++ Tests/Surfaces/LineSurfaceTests.cpp | 7 ++ Tests/Surfaces/PlaneSurfaceTests.cpp | 6 ++ 15 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 Core/include/ACTS/Utilities/ThrowAssert.hpp diff --git a/Core/include/ACTS/Utilities/BinningData.hpp b/Core/include/ACTS/Utilities/BinningData.hpp index 1b63dab01..edc4cbd57 100644 --- a/Core/include/ACTS/Utilities/BinningData.hpp +++ b/Core/include/ACTS/Utilities/BinningData.hpp @@ -19,6 +19,7 @@ #include <vector> #include "ACTS/Utilities/BinningType.hpp" #include "ACTS/Utilities/Definitions.hpp" +#include "ACTS/Utilities/ThrowAssert.hpp" namespace Acts { @@ -145,7 +146,7 @@ public: , m_functionPtr(nullptr) { // assert a no-size case - assert(m_boundaries.size() > 1); + throw_assert(m_boundaries.size() > 1, "Must have more than one boundary"); min = m_boundaries[0]; max = m_boundaries[m_boundaries.size() - 1]; // set to equidistant search @@ -373,7 +374,8 @@ public: search(float value) const { if (zdim) return 0; - assert(m_functionPtr != nullptr); + throw_assert(m_functionPtr != nullptr, + "Search function pointer is nullptr"); return (!subBinningData) ? (*m_functionPtr)(value, *this) : searchWithSubStructure(value); } diff --git a/Core/include/ACTS/Utilities/ThrowAssert.hpp b/Core/include/ACTS/Utilities/ThrowAssert.hpp new file mode 100644 index 000000000..c52e814fa --- /dev/null +++ b/Core/include/ACTS/Utilities/ThrowAssert.hpp @@ -0,0 +1,90 @@ +// This file is part of the ACTS project. +// +// Copyright (C) 2018 ACTS project team +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef ACTS_UTILITIES_THROWASSERT_H +#define ACTS_UTILITIES_THROWASSERT_H 1 + +#include <exception> +#include <iostream> +#include <sstream> +#include <string> + +namespace Acts { +/// @brief Exception type for assertion failures +/// This class captures the information available to the throw_assert macro +class AssertionFailureException : public std::exception +{ +public: + /// @brief Class which allows to use the << operator to assemble a string + class StreamFormatter + { + private: + std::ostringstream stream; + + public: + /// @brief Converts to string + operator std::string() const { return stream.str(); } + + /// @brief Stream operator which takes everything and forwards + /// it to the stringstream. + /// @tparam T type of anything + /// @param value const ref to anything + template <typename T> + StreamFormatter& + operator<<(const T& value) + { + stream << value; + return *this; + } + }; + + /// @brief Construct an assertion failure exception, captures macro info + /// @param expression The expression being asserted + /// @param file The current file + /// @param line The current line + /// @param msg The message to print if assertion fails + AssertionFailureException(std::string expression, + std::string file, + int line, + const std::string& msg) + { + std::ostringstream os; + + if (!msg.empty()) { + os << msg << ": "; + } + + os << "Assertion '" << expression << "'"; + + os << " failed in file '" << file << "' line " << line; + report = os.str(); + } + + /// The assertion message + virtual const char* + what() const throw() + { + return report.c_str(); + } + +private: + std::string report; +}; + +} // namespace Acts + +#define throw_assert(EXPRESSION, MESSAGE) \ + if (!(EXPRESSION)) { \ + throw Acts::AssertionFailureException( \ + #EXPRESSION, \ + __FILE__, \ + __LINE__, \ + (Acts::AssertionFailureException::StreamFormatter() << MESSAGE)); \ + } + +#endif diff --git a/Core/src/Surfaces/ConeSurface.cpp b/Core/src/Surfaces/ConeSurface.cpp index 1f410b66a..96858a9ed 100644 --- a/Core/src/Surfaces/ConeSurface.cpp +++ b/Core/src/Surfaces/ConeSurface.cpp @@ -17,6 +17,7 @@ #include <iomanip> #include <iostream> +#include "ACTS/Utilities/ThrowAssert.hpp" #include "ACTS/Utilities/detail/RealQuadraticEquation.hpp" Acts::ConeSurface::ConeSurface(const ConeSurface& other) @@ -52,7 +53,7 @@ Acts::ConeSurface::ConeSurface(std::shared_ptr<const Transform3D> htrans, std::shared_ptr<const ConeBounds> cbounds) : Surface(htrans), m_bounds(cbounds) { - assert(cbounds); + throw_assert(cbounds, "ConeBounds must not be nullptr"); } Acts::ConeSurface::~ConeSurface() diff --git a/Core/src/Surfaces/CylinderSurface.cpp b/Core/src/Surfaces/CylinderSurface.cpp index 38b5243fc..ab502c3e4 100644 --- a/Core/src/Surfaces/CylinderSurface.cpp +++ b/Core/src/Surfaces/CylinderSurface.cpp @@ -17,6 +17,7 @@ #include <iomanip> #include <iostream> +#include "ACTS/Utilities/ThrowAssert.hpp" #include "ACTS/Utilities/detail/RealQuadraticEquation.hpp" Acts::CylinderSurface::CylinderSurface(const CylinderSurface& other) @@ -64,7 +65,7 @@ Acts::CylinderSurface::CylinderSurface( std::shared_ptr<const CylinderBounds> cbounds) : Surface(htrans), m_bounds(cbounds) { - assert(cbounds); + throw_assert(cbounds, "CylinderBounds must not be nullptr"); } Acts::CylinderSurface::~CylinderSurface() diff --git a/Core/src/Surfaces/DiamondBounds.cpp b/Core/src/Surfaces/DiamondBounds.cpp index 6beb1e265..7713f5d8b 100644 --- a/Core/src/Surfaces/DiamondBounds.cpp +++ b/Core/src/Surfaces/DiamondBounds.cpp @@ -11,6 +11,7 @@ /////////////////////////////////////////////////////////////////// #include "ACTS/Surfaces/DiamondBounds.hpp" +#include "ACTS/Utilities/ThrowAssert.hpp" #include <cmath> #include <iomanip> @@ -29,8 +30,8 @@ Acts::DiamondBounds::DiamondBounds(double minhalex, , m_boundingBox(std::max(std::max(minhalex, medhalex), maxhalex), std::max(haley1, haley2)) { - assert((minhalex <= medhalex) && "Hexagon must be a convex polygon"); - assert((maxhalex <= medhalex) && "Hexagon must be a convex polygon"); + throw_assert((minhalex <= medhalex), "Hexagon must be a convex polygon"); + throw_assert((maxhalex <= medhalex), "Hexagon must be a convex polygon"); } Acts::DiamondBounds::~DiamondBounds() diff --git a/Core/src/Surfaces/DiscSurface.cpp b/Core/src/Surfaces/DiscSurface.cpp index 0871129e5..0dc1a097a 100644 --- a/Core/src/Surfaces/DiscSurface.cpp +++ b/Core/src/Surfaces/DiscSurface.cpp @@ -20,6 +20,7 @@ #include "ACTS/Surfaces/InfiniteBounds.hpp" #include "ACTS/Surfaces/RadialBounds.hpp" #include "ACTS/Utilities/Definitions.hpp" +#include "ACTS/Utilities/ThrowAssert.hpp" Acts::DiscSurface::DiscSurface(const DiscSurface& other) : Surface(other), m_bounds(other.m_bounds) @@ -69,7 +70,7 @@ Acts::DiscSurface::DiscSurface(std::shared_ptr<const DiscBounds> dbounds, const Identifier& identifier) : Surface(detelement, identifier), m_bounds(nullptr) { - assert(dbounds); + throw_assert(dbounds, "nullptr as DiscBounds"); } Acts::DiscSurface::~DiscSurface() diff --git a/Core/src/Surfaces/LineSurface.cpp b/Core/src/Surfaces/LineSurface.cpp index 802bdf536..6dd3f7898 100644 --- a/Core/src/Surfaces/LineSurface.cpp +++ b/Core/src/Surfaces/LineSurface.cpp @@ -16,6 +16,8 @@ #include <iomanip> #include <iostream> +#include "ACTS/Utilities/ThrowAssert.hpp" + Acts::LineSurface::LineSurface(std::shared_ptr<const Transform3D> htrans, double radius, double halez) @@ -34,7 +36,7 @@ Acts::LineSurface::LineSurface(std::shared_ptr<const LineBounds> lbounds, const Identifier& id) : Surface(detelement, id), m_bounds(lbounds) { - assert(lbounds); + throw_assert(lbounds, "LineBounds must not be nullptr"); } Acts::LineSurface::LineSurface(const LineSurface& other) diff --git a/Core/src/Surfaces/PlaneSurface.cpp b/Core/src/Surfaces/PlaneSurface.cpp index aae894408..1f89f90e0 100644 --- a/Core/src/Surfaces/PlaneSurface.cpp +++ b/Core/src/Surfaces/PlaneSurface.cpp @@ -19,6 +19,7 @@ #include "ACTS/Surfaces/InfiniteBounds.hpp" #include "ACTS/Surfaces/RectangleBounds.hpp" #include "ACTS/Utilities/Identifier.hpp" +#include "ACTS/Utilities/ThrowAssert.hpp" Acts::PlaneSurface::PlaneSurface(const PlaneSurface& other) : Surface(other), m_bounds(other.m_bounds) @@ -60,7 +61,7 @@ Acts::PlaneSurface::PlaneSurface(std::shared_ptr<const PlanarBounds> pbounds, : Surface(detelement, identifier), m_bounds(pbounds) { /// surfaces representing a detector element must have bounds - assert(pbounds); + throw_assert(pbounds, "PlaneBounds must not be nullptr"); } Acts::PlaneSurface::PlaneSurface(std::shared_ptr<const Transform3D> htrans, diff --git a/Tests/Surfaces/ConeSurfaceTests.cpp b/Tests/Surfaces/ConeSurfaceTests.cpp index 3640db95d..9ab124c93 100644 --- a/Tests/Surfaces/ConeSurfaceTests.cpp +++ b/Tests/Surfaces/ConeSurfaceTests.cpp @@ -71,6 +71,10 @@ namespace Test { /// Copied and transformed ConeSurface copiedTransformedConeSurface(coneSurfaceObject, *pTransform); BOOST_TEST(copiedTransformedConeSurface.type() == Surface::Cone); + + /// Construct with nullptr bounds + BOOST_CHECK_THROW(ConeSurface nullBounds(nullptr, nullptr), + AssertionFailureException); } // /// Unit test for testing ConeSurface properties diff --git a/Tests/Surfaces/CylinderSurfaceTests.cpp b/Tests/Surfaces/CylinderSurfaceTests.cpp index 244c33622..86391bae3 100644 --- a/Tests/Surfaces/CylinderSurfaceTests.cpp +++ b/Tests/Surfaces/CylinderSurfaceTests.cpp @@ -68,6 +68,10 @@ namespace Test { CylinderSurface copiedTransformedCylinderSurface(cylinderSurfaceObject, *pTransform); BOOST_TEST(copiedTransformedCylinderSurface.type() == Surface::Cylinder); + + /// Construct with nullptr bounds + BOOST_CHECK_THROW(CylinderSurface nullBounds(nullptr, nullptr), + AssertionFailureException); } // /// Unit test for testing CylinderSurface properties diff --git a/Tests/Surfaces/DetectorElementStub.hpp b/Tests/Surfaces/DetectorElementStub.hpp index 10d833dfa..047e03ec8 100644 --- a/Tests/Surfaces/DetectorElementStub.hpp +++ b/Tests/Surfaces/DetectorElementStub.hpp @@ -38,6 +38,8 @@ class LineBounds; class DetectorElementStub : public DetectorElementBase { public: + DetectorElementStub() : DetectorElementBase() {} + /// Constructor for single sided detector element /// - bound to a Plane Surface /// diff --git a/Tests/Surfaces/DiamondBoundsTests.cpp b/Tests/Surfaces/DiamondBoundsTests.cpp index 627250b12..b0b2fb3c3 100644 --- a/Tests/Surfaces/DiamondBoundsTests.cpp +++ b/Tests/Surfaces/DiamondBoundsTests.cpp @@ -44,6 +44,14 @@ namespace Test { DiamondBounds original(minHalfX, midHalfX, maxHalfX, halfY1, halfY2); DiamondBounds copied(original); BOOST_TEST(copied.type() == SurfaceBounds::Diamond); + + // invalid inputs + BOOST_CHECK_THROW( + DiamondBounds db(midHalfX, minHalfX, maxHalfX, halfY1, halfY2), + AssertionFailureException); + BOOST_CHECK_THROW( + DiamondBounds db(minHalfX, maxHalfX, midHalfX, halfY1, halfY2), + AssertionFailureException); } /// Unit tests for DiamondBounds properties BOOST_AUTO_TEST_CASE(DiamondBoundsProperties) diff --git a/Tests/Surfaces/DiscSurfaceTests.cpp b/Tests/Surfaces/DiscSurfaceTests.cpp index 797598b76..d239397d4 100644 --- a/Tests/Surfaces/DiscSurfaceTests.cpp +++ b/Tests/Surfaces/DiscSurfaceTests.cpp @@ -64,6 +64,12 @@ namespace Test { // /// Copied and transformed DiscSurface BOOST_CHECK_NO_THROW(DiscSurface(anotherDiscSurface, *pTransform)); + + /// Construct with nullptr bounds + Identifier id; + DetectorElementStub detElem; + BOOST_CHECK_THROW(DiscSurface nullBounds(nullptr, detElem, id), + AssertionFailureException); } /// Unit tests of all named methods diff --git a/Tests/Surfaces/LineSurfaceTests.cpp b/Tests/Surfaces/LineSurfaceTests.cpp index e6093f2cc..eeb5a3acc 100644 --- a/Tests/Surfaces/LineSurfaceTests.cpp +++ b/Tests/Surfaces/LineSurfaceTests.cpp @@ -65,6 +65,13 @@ namespace Test { BOOST_CHECK(LineSurfaceStub(lineToCopy).constructedOk()); // Copied and transformed ctor BOOST_CHECK(LineSurfaceStub(lineToCopy, transform).constructedOk()); + + /// Construct with nullptr bounds + Identifier id; + DetectorElementStub detElem; + BOOST_CHECK_THROW(LineSurfaceStub nullBounds(nullptr, detElem, id), + AssertionFailureException); + BOOST_TEST_MESSAGE( "All LineSurface constructors are callable without problem"); } diff --git a/Tests/Surfaces/PlaneSurfaceTests.cpp b/Tests/Surfaces/PlaneSurfaceTests.cpp index d8c283994..95ea58f7c 100644 --- a/Tests/Surfaces/PlaneSurfaceTests.cpp +++ b/Tests/Surfaces/PlaneSurfaceTests.cpp @@ -54,6 +54,12 @@ namespace Test { /// Copied and transformed PlaneSurface copiedTransformedPlaneSurface(PlaneSurfaceObject, *pTransform); BOOST_TEST(copiedTransformedPlaneSurface.type() == Surface::Plane); + + /// Construct with nullptr bounds + Identifier id; + DetectorElementStub detElem; + BOOST_CHECK_THROW(PlaneSurface nullBounds(nullptr, detElem, id), + AssertionFailureException); } // /// Unit test for testing PlaneSurface properties -- GitLab