From 9d8222ad667284cfad8be9cb27b25dab75f34ccb Mon Sep 17 00:00:00 2001 From: trzeci Date: Tue, 17 Dec 2019 23:43:31 +0100 Subject: Disable move constructor and assignment operator for test classes. Disable move operations for TEST() and TEST_F() macros. Previous implementation disabled only copy ctor and assing operator, but this was violating rule of 5[1], which was captured by static code analysis tools like clang-tidy `cppcoreguidelines-special-member-functions`. [1]: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all --- googletest/include/gtest/internal/gtest-internal.h | 2 ++ googletest/include/gtest/internal/gtest-port.h | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index eac831a..062611e 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -1395,6 +1395,8 @@ constexpr bool InstantiateTypedTestCase_P_IsDeprecated() { return true; } static ::testing::TestInfo* const test_info_ GTEST_ATTRIBUTE_UNUSED_; \ GTEST_DISALLOW_COPY_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ test_name)); \ + GTEST_DISALLOW_MOVE_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ + test_name)); \ }; \ \ ::testing::TestInfo* const GTEST_TEST_CLASS_NAME_(test_suite_name, \ diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h index fcaac0b..95a9479 100644 --- a/googletest/include/gtest/internal/gtest-port.h +++ b/googletest/include/gtest/internal/gtest-port.h @@ -190,8 +190,10 @@ // GTEST_AMBIGUOUS_ELSE_BLOCKER_ - for disabling a gcc warning. // GTEST_ATTRIBUTE_UNUSED_ - declares that a class' instances or a // variable don't have to be used. -// GTEST_DISALLOW_ASSIGN_ - disables operator=. +// GTEST_DISALLOW_ASSIGN_ - disables copy operator=. // GTEST_DISALLOW_COPY_AND_ASSIGN_ - disables copy ctor and operator=. +// GTEST_DISALLOW_MOVE_ASSIGN_ - disables move operator=. +// GTEST_DISALLOW_MOVE_AND_ASSIGN_ - disables move ctor and operator=. // GTEST_MUST_USE_RESULT_ - declares that a function's result must be used. // GTEST_INTENTIONAL_CONST_COND_PUSH_ - start code section where MSVC C4127 is // suppressed (constant conditional). @@ -666,7 +668,7 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; #endif -// A macro to disallow operator= +// A macro to disallow copy operator= // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_ASSIGN_(type) \ void operator=(type const &) = delete @@ -677,6 +679,17 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; type(type const &) = delete; \ GTEST_DISALLOW_ASSIGN_(type) +// A macro to disallow move operator= +// This should be used in the private: declarations for a class. +#define GTEST_DISALLOW_MOVE_ASSIGN_(type) \ + void operator=(type &&) = delete + +// A macro to disallow move constructor and operator= +// This should be used in the private: declarations for a class. +#define GTEST_DISALLOW_MOVE_AND_ASSIGN_(type) \ + type(type &&) = delete; \ + GTEST_DISALLOW_MOVE_ASSIGN_(type) + // Tell the compiler to warn about unused return values for functions declared // with this macro. The macro should be used on function declarations // following the argument list: -- cgit v0.12 From 77b3a250ea9e7c6d01b6eba1dfe70568b7e1fabc Mon Sep 17 00:00:00 2001 From: "Piotr Paczkowski (trzeci.eu)" Date: Fri, 20 Dec 2019 09:39:06 +0100 Subject: Review notes: Return T& from assignment operators --- googletest/include/gtest/internal/gtest-port.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h index 95a9479..1a7f80c 100644 --- a/googletest/include/gtest/internal/gtest-port.h +++ b/googletest/include/gtest/internal/gtest-port.h @@ -671,7 +671,7 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; // A macro to disallow copy operator= // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_ASSIGN_(type) \ - void operator=(type const &) = delete + type& operator=(type const &) = delete // A macro to disallow copy constructor and operator= // This should be used in the private: declarations for a class. @@ -682,7 +682,7 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; // A macro to disallow move operator= // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_MOVE_ASSIGN_(type) \ - void operator=(type &&) = delete + type& operator=(type &&) = delete // A macro to disallow move constructor and operator= // This should be used in the private: declarations for a class. -- cgit v0.12 From 05701fee2896329009cb040496bd0d6714c9a78b Mon Sep 17 00:00:00 2001 From: "Piotr Paczkowski (trzeci.eu)" Date: Fri, 20 Dec 2019 09:41:58 +0100 Subject: Deleted functions as part of public interface --- googletest/include/gtest/internal/gtest-internal.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index 062611e..ce081aa 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -1389,14 +1389,14 @@ constexpr bool InstantiateTypedTestCase_P_IsDeprecated() { return true; } : public parent_class { \ public: \ GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() {} \ - \ - private: \ - void TestBody() override; \ - static ::testing::TestInfo* const test_info_ GTEST_ATTRIBUTE_UNUSED_; \ GTEST_DISALLOW_COPY_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ test_name)); \ GTEST_DISALLOW_MOVE_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ test_name)); \ + \ + private: \ + void TestBody() override; \ + static ::testing::TestInfo* const test_info_ GTEST_ATTRIBUTE_UNUSED_; \ }; \ \ ::testing::TestInfo* const GTEST_TEST_CLASS_NAME_(test_suite_name, \ -- cgit v0.12 From cc05a3ca014bdc25eaf987672004b36fa1ceeb36 Mon Sep 17 00:00:00 2001 From: "Piotr Paczkowski (trzeci.eu)" Date: Fri, 20 Dec 2019 09:45:28 +0100 Subject: Define default destructor for test classes --- googletest/include/gtest/internal/gtest-internal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index ce081aa..ff2ff4b 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -1389,6 +1389,7 @@ constexpr bool InstantiateTypedTestCase_P_IsDeprecated() { return true; } : public parent_class { \ public: \ GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() {} \ + ~GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() = default; \ GTEST_DISALLOW_COPY_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ test_name)); \ GTEST_DISALLOW_MOVE_AND_ASSIGN_(GTEST_TEST_CLASS_NAME_(test_suite_name, \ -- cgit v0.12 From bf31ed376ab1ae181dfa2bb0383f7710229b0d14 Mon Sep 17 00:00:00 2001 From: "Piotr Paczkowski (trzeci.eu)" Date: Fri, 20 Dec 2019 09:51:35 +0100 Subject: Make move operation noexcept. --- googletest/include/gtest/internal/gtest-port.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h index 1a7f80c..0543da5 100644 --- a/googletest/include/gtest/internal/gtest-port.h +++ b/googletest/include/gtest/internal/gtest-port.h @@ -682,12 +682,12 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; // A macro to disallow move operator= // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_MOVE_ASSIGN_(type) \ - type& operator=(type &&) = delete + type& operator=(type &&) noexcept = delete // A macro to disallow move constructor and operator= // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_MOVE_AND_ASSIGN_(type) \ - type(type &&) = delete; \ + type(type &&) noexcept = delete; \ GTEST_DISALLOW_MOVE_ASSIGN_(type) // Tell the compiler to warn about unused return values for functions declared -- cgit v0.12