From 4211b65fc7d0fc5de523faceea6719195a5a0d0a Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 10:59:10 -0800 Subject: [PATCH 1/9] Add functions for C++ assertions Signed-off-by: Zachary Michaels --- CMakeLists.txt | 8 +++++ include/rcpputils/asserts.hpp | 55 +++++++++++++++++++++++++++++++++++ test/test_asserts.cpp | 44 ++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 include/rcpputils/asserts.hpp create mode 100644 test/test_asserts.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 7965947..5431765 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,6 +46,14 @@ if(BUILD_TESTING) ament_lint_auto_find_test_dependencies() + ament_add_gtest(test_asserts_ndebug test/test_asserts.cpp) + + if(TARGET test_asserts_ndebug) + add_definitions(-DNDEBUG) + endif() + + ament_add_gtest(test_asserts_debug test/test_asserts.cpp) + ament_add_gtest(test_thread_safety_annotations test/test_thread_safety_annotations.cpp) ament_add_gtest(test_join test/test_join.cpp) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp new file mode 100644 index 0000000..851fea2 --- /dev/null +++ b/include/rcpputils/asserts.hpp @@ -0,0 +1,55 @@ +// Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RCPPUTILS__ASSERTS_HPP_ +#define RCPPUTILS__ASSERTS_HPP_ + +#include + +namespace rcpputils +{ +/** + * Checks that a state condition passes. + * + * \param condition + * \throw std::runtime_error if the condition is not met. + */ +inline void check(bool condition) +{ + if (!condition) { + throw std::runtime_error{"Check reported invalid state!"}; + } +} + +/** + * Asserts that a condition passes. + * + * This function behaves similar to assert, except that it throws instead of invoking abort(). + * \param condition + * \throw std::runtime_error if the macro NDEBUG is not set and the condition is not met. + */ +inline void ros_assert(bool condition) +{ +// Same macro definition used by cassert +#ifndef NDEBUG + if (!condition) { + throw std::runtime_error{"Assertion failed!"}; + } +#else + (void) condition; +#endif +} +} // namespace rcpputils + +#endif // RCPPUTILS__ASSERTS_HPP_ diff --git a/test/test_asserts.cpp b/test/test_asserts.cpp new file mode 100644 index 0000000..61214fa --- /dev/null +++ b/test/test_asserts.cpp @@ -0,0 +1,44 @@ +// Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "gtest/gtest.h" + +#include "rcpputils/asserts.hpp" + +TEST(test_asserts, check_throws_if_condition_is_false) { + EXPECT_THROW(rcpputils::check(false), std::runtime_error); +} + +TEST(test_asserts, check_does_not_throw_if_condition_is_true) { + EXPECT_NO_THROW(rcpputils::check(true)); +} + +#ifndef NDEBUG +TEST(test_asserts, ros_assert_throws_if_condition_is_false_and_ndebug_not_set) { + EXPECT_THROW(rcpputils::ros_assert(false), std::runtime_error); +} + +TEST(test_asserts, ros_assert_does_not_throw_if_condition_is_true_and_ndebug_not_set) +{ + EXPECT_NO_THROW(rcpputils::ros_assert(true)); +} + +#else +TEST(test_asserts, ros_assert_does_not_throw_if_ndebug_set) { + EXPECT_NO_THROW(rcpputils::ros_assert(false)); + EXPECT_NO_THROW(rcpputils::ros_assert(true)); +} +#endif From 43de3126314560de9fcfc8ed09eed0cbf782d4d9 Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 14:08:12 -0800 Subject: [PATCH 2/9] Rename functions to assert_true and check_true Signed-off-by: Zachary Michaels --- include/rcpputils/asserts.hpp | 4 ++-- test/test_asserts.cpp | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index 851fea2..e93266b 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -25,7 +25,7 @@ namespace rcpputils * \param condition * \throw std::runtime_error if the condition is not met. */ -inline void check(bool condition) +inline void check_true(bool condition) { if (!condition) { throw std::runtime_error{"Check reported invalid state!"}; @@ -39,7 +39,7 @@ inline void check(bool condition) * \param condition * \throw std::runtime_error if the macro NDEBUG is not set and the condition is not met. */ -inline void ros_assert(bool condition) +inline void assert_true(bool condition) { // Same macro definition used by cassert #ifndef NDEBUG diff --git a/test/test_asserts.cpp b/test/test_asserts.cpp index 61214fa..9e60e83 100644 --- a/test/test_asserts.cpp +++ b/test/test_asserts.cpp @@ -19,26 +19,26 @@ #include "rcpputils/asserts.hpp" TEST(test_asserts, check_throws_if_condition_is_false) { - EXPECT_THROW(rcpputils::check(false), std::runtime_error); + EXPECT_THROW(rcpputils::check_true(false), std::runtime_error); } TEST(test_asserts, check_does_not_throw_if_condition_is_true) { - EXPECT_NO_THROW(rcpputils::check(true)); + EXPECT_NO_THROW(rcpputils::check_true(true)); } #ifndef NDEBUG TEST(test_asserts, ros_assert_throws_if_condition_is_false_and_ndebug_not_set) { - EXPECT_THROW(rcpputils::ros_assert(false), std::runtime_error); + EXPECT_THROW(rcpputils::assert_true(false), std::runtime_error); } TEST(test_asserts, ros_assert_does_not_throw_if_condition_is_true_and_ndebug_not_set) { - EXPECT_NO_THROW(rcpputils::ros_assert(true)); + EXPECT_NO_THROW(rcpputils::assert_true(true)); } #else TEST(test_asserts, ros_assert_does_not_throw_if_ndebug_set) { - EXPECT_NO_THROW(rcpputils::ros_assert(false)); - EXPECT_NO_THROW(rcpputils::ros_assert(true)); + EXPECT_NO_THROW(rcpputils::assert_true(false)); + EXPECT_NO_THROW(rcpputils::assert_true(true)); } #endif From ca530ce8582912de14936c48ef513602f5778dbc Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 14:29:47 -0800 Subject: [PATCH 3/9] Use target_compile_definitions in CMakeLists Signed-off-by: Zachary Michaels --- CMakeLists.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5431765..e93f90c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -48,9 +48,7 @@ if(BUILD_TESTING) ament_add_gtest(test_asserts_ndebug test/test_asserts.cpp) - if(TARGET test_asserts_ndebug) - add_definitions(-DNDEBUG) - endif() + target_compile_definitions(test_asserts_ndebug PUBLIC NDEBUG) ament_add_gtest(test_asserts_debug test/test_asserts.cpp) From e378655fcec5f2dd3e4a107607b9cad4686f72b6 Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 14:31:11 -0800 Subject: [PATCH 4/9] Use custom exceptions for better testing Signed-off-by: Zachary Michaels --- CMakeLists.txt | 3 +++ include/rcpputils/asserts.hpp | 34 +++++++++++++++++++++++++++++----- src/asserts.cpp | 28 ++++++++++++++++++++++++++++ test/test_asserts.cpp | 4 ++-- 4 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 src/asserts.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index e93f90c..f6a4a75 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,7 @@ include_directories(include) ament_export_include_directories(include) add_library(${PROJECT_NAME} + src/asserts.cpp src/find_library.cpp) target_include_directories(${PROJECT_NAME} PUBLIC @@ -47,10 +48,12 @@ if(BUILD_TESTING) ament_lint_auto_find_test_dependencies() ament_add_gtest(test_asserts_ndebug test/test_asserts.cpp) + target_link_libraries(test_asserts_ndebug ${PROJECT_NAME}) target_compile_definitions(test_asserts_ndebug PUBLIC NDEBUG) ament_add_gtest(test_asserts_debug test/test_asserts.cpp) + target_link_libraries(test_asserts_debug ${PROJECT_NAME}) ament_add_gtest(test_thread_safety_annotations test/test_thread_safety_annotations.cpp) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index e93266b..db859b6 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -15,20 +15,44 @@ #ifndef RCPPUTILS__ASSERTS_HPP_ #define RCPPUTILS__ASSERTS_HPP_ -#include +#include +#include namespace rcpputils { + +class AssertionException : public std::exception +{ + std::string msg_; + +public: + explicit AssertionException(const std::string & msg) + : msg_{msg} {} + + virtual const char * what() const throw(); +}; + +class IllegalStateException : public std::exception +{ + std::string msg_; + +public: + explicit IllegalStateException(const std::string & msg) + : msg_{msg} {} + + virtual const char * what() const throw(); +}; + /** * Checks that a state condition passes. * * \param condition - * \throw std::runtime_error if the condition is not met. + * \throw rcpputils::IllegalStateException if the condition is not met. */ inline void check_true(bool condition) { if (!condition) { - throw std::runtime_error{"Check reported invalid state!"}; + throw rcpputils::IllegalStateException{"Check reported invalid state!"}; } } @@ -37,14 +61,14 @@ inline void check_true(bool condition) * * This function behaves similar to assert, except that it throws instead of invoking abort(). * \param condition - * \throw std::runtime_error if the macro NDEBUG is not set and the condition is not met. + * \throw rcpputils::AssertionException if the macro NDEBUG is not set and the condition is not met. */ inline void assert_true(bool condition) { // Same macro definition used by cassert #ifndef NDEBUG if (!condition) { - throw std::runtime_error{"Assertion failed!"}; + throw rcpputils::AssertionException{"Assertion failed!"}; } #else (void) condition; diff --git a/src/asserts.cpp b/src/asserts.cpp new file mode 100644 index 0000000..e553fdf --- /dev/null +++ b/src/asserts.cpp @@ -0,0 +1,28 @@ +// Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "rcpputils/asserts.hpp" + +namespace rcpputils +{ +const char * AssertionException::what() const throw() +{ + return msg_.c_str(); +} + +const char * IllegalStateException::what() const throw() +{ + return msg_.c_str(); +} +} // namespace rcpputils diff --git a/test/test_asserts.cpp b/test/test_asserts.cpp index 9e60e83..a2aa144 100644 --- a/test/test_asserts.cpp +++ b/test/test_asserts.cpp @@ -19,7 +19,7 @@ #include "rcpputils/asserts.hpp" TEST(test_asserts, check_throws_if_condition_is_false) { - EXPECT_THROW(rcpputils::check_true(false), std::runtime_error); + EXPECT_THROW(rcpputils::check_true(false), rcpputils::IllegalStateException); } TEST(test_asserts, check_does_not_throw_if_condition_is_true) { @@ -28,7 +28,7 @@ TEST(test_asserts, check_does_not_throw_if_condition_is_true) { #ifndef NDEBUG TEST(test_asserts, ros_assert_throws_if_condition_is_false_and_ndebug_not_set) { - EXPECT_THROW(rcpputils::assert_true(false), std::runtime_error); + EXPECT_THROW(rcpputils::assert_true(false), rcpputils::AssertionException); } TEST(test_asserts, ros_assert_does_not_throw_if_condition_is_true_and_ndebug_not_set) From 69cce050716594185ee393feaf14f11bb792b0af Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 14:36:29 -0800 Subject: [PATCH 5/9] Add require_true and tests for require_true Signed-off-by: Zachary Michaels --- include/rcpputils/asserts.hpp | 14 ++++++++++++++ test/test_asserts.cpp | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index db859b6..1c59114 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -16,6 +16,7 @@ #define RCPPUTILS__ASSERTS_HPP_ #include +#include #include namespace rcpputils @@ -43,6 +44,19 @@ class IllegalStateException : public std::exception virtual const char * what() const throw(); }; +/** + * Checks that an argument condition passes. + * + * \param condition + * \throw std::invalid_argument if the condition is not met. + */ +inline void require_true(bool condition) +{ + if (!condition) { + throw std::invalid_argument{"Invalid argument passed!"}; + } +} + /** * Checks that a state condition passes. * diff --git a/test/test_asserts.cpp b/test/test_asserts.cpp index a2aa144..2fc58d1 100644 --- a/test/test_asserts.cpp +++ b/test/test_asserts.cpp @@ -18,6 +18,14 @@ #include "rcpputils/asserts.hpp" +TEST(test_asserts, require_throws_if_condition_is_false) { + EXPECT_THROW(rcpputils::require_true(false), std::invalid_argument); +} + +TEST(test_asserts, require_does_not_throw_if_condition_is_true) { + EXPECT_NO_THROW(rcpputils::require_true(true)); +} + TEST(test_asserts, check_throws_if_condition_is_false) { EXPECT_THROW(rcpputils::check_true(false), rcpputils::IllegalStateException); } From 44aec4c301cb3150fa36972b577743b95e9509f9 Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 14:48:22 -0800 Subject: [PATCH 6/9] Check for target_asserts_ndebug before adding target_compile_definitions Signed-off-by: Zachary Michaels --- CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f6a4a75..b6f5414 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,7 +50,9 @@ if(BUILD_TESTING) ament_add_gtest(test_asserts_ndebug test/test_asserts.cpp) target_link_libraries(test_asserts_ndebug ${PROJECT_NAME}) - target_compile_definitions(test_asserts_ndebug PUBLIC NDEBUG) + if(TARGET test_asserts_ndebug) + target_compile_definitions(test_asserts_ndebug PUBLIC NDEBUG) + endif() ament_add_gtest(test_asserts_debug test/test_asserts.cpp) target_link_libraries(test_asserts_debug ${PROJECT_NAME}) From 31b38a6739aeca49a5bf345b82e58e360e685566 Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 15:07:37 -0800 Subject: [PATCH 7/9] Add RCPPUTILS_PUBLIC on AssertionException and IllegalStateException Signed-off-by: Zachary Michaels --- include/rcpputils/asserts.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index 1c59114..e83b56f 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -19,10 +19,12 @@ #include #include +#include "rcpputils/visibility_control.hpp" + namespace rcpputils { -class AssertionException : public std::exception +class RCPPUTILS_PUBLIC AssertionException : public std::exception { std::string msg_; @@ -33,7 +35,7 @@ class AssertionException : public std::exception virtual const char * what() const throw(); }; -class IllegalStateException : public std::exception +class RCPPUTILS_PUBLIC IllegalStateException : public std::exception { std::string msg_; From c3db1bcea52083d73c097f0e3b917d56ffbeb4ff Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 15:28:16 -0800 Subject: [PATCH 8/9] Ignore C4275 warning due to exporting class Signed-off-by: Zachary Michaels --- include/rcpputils/asserts.hpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index e83b56f..afd22d7 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -21,6 +21,14 @@ #include "rcpputils/visibility_control.hpp" +// Needed to disable compiler warning for exporting a class that extends a +// non-DLL-interface class. +// This should be fine since its extending an STL class. +#ifdef _WIN32 +# pragma warning(push) +# pragma warning(disable:4275) +#endif + namespace rcpputils { From daa06b81d1a8173b9945a4d517ea8157d05dad1d Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 23 Jan 2020 15:58:03 -0800 Subject: [PATCH 9/9] Disable 4251 and 4275, pop warnings, and use const char * for ctor Signed-off-by: Zachary Michaels --- include/rcpputils/asserts.hpp | 11 +++++++---- src/asserts.cpp | 10 ++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/include/rcpputils/asserts.hpp b/include/rcpputils/asserts.hpp index afd22d7..d56f0ca 100644 --- a/include/rcpputils/asserts.hpp +++ b/include/rcpputils/asserts.hpp @@ -26,6 +26,7 @@ // This should be fine since its extending an STL class. #ifdef _WIN32 # pragma warning(push) +# pragma warning(disable:4251) # pragma warning(disable:4275) #endif @@ -37,8 +38,7 @@ class RCPPUTILS_PUBLIC AssertionException : public std::exception std::string msg_; public: - explicit AssertionException(const std::string & msg) - : msg_{msg} {} + explicit AssertionException(const char * msg); virtual const char * what() const throw(); }; @@ -48,8 +48,7 @@ class RCPPUTILS_PUBLIC IllegalStateException : public std::exception std::string msg_; public: - explicit IllegalStateException(const std::string & msg) - : msg_{msg} {} + explicit IllegalStateException(const char * msg); virtual const char * what() const throw(); }; @@ -100,4 +99,8 @@ inline void assert_true(bool condition) } } // namespace rcpputils +#ifdef _WIN32 +# pragma warning(pop) +#endif + #endif // RCPPUTILS__ASSERTS_HPP_ diff --git a/src/asserts.cpp b/src/asserts.cpp index e553fdf..c072ba1 100644 --- a/src/asserts.cpp +++ b/src/asserts.cpp @@ -16,11 +16,21 @@ namespace rcpputils { +AssertionException::AssertionException(const char * msg) +{ + msg_ = msg; +} + const char * AssertionException::what() const throw() { return msg_.c_str(); } +IllegalStateException::IllegalStateException(const char * msg) +{ + msg_ = msg; +} + const char * IllegalStateException::what() const throw() { return msg_.c_str();