From 766ac2e1a413e87d42d67e3286c70f0af4853679 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Thu, 16 Apr 2020 15:52:17 -0400 Subject: Remove all uses of GTEST_DISALLOW_{MOVE_,}ASSIGN_. None of these are strictly needed for correctness. A large number of them (maybe all of them?) trigger `-Wdeprecated` warnings on Clang trunk as soon as you try to use the implicitly defaulted (but deprecated) copy constructor of a class that has deleted its copy assignment operator. By declaring a deleted copy assignment operator, the old code also caused the move constructor and move assignment operator to be non-declared. This means that the old code never got move semantics -- "move-construction" would simply call the defaulted (but deprecated) copy constructor instead. With the new code, "move-construction" calls the defaulted move constructor, which I believe is what we want to happen. So this is a runtime performance optimization. Unfortunately we can't yet physically remove the definitions of these macros from gtest-port.h, because they are being used by other code internally at Google (according to zhangxy988). But no new uses should be added going forward. --- googletest/include/gtest/internal/gtest-internal.h | 2 -- googletest/include/gtest/internal/gtest-port.h | 6 ++---- googletest/test/googletest-port-test.cc | 2 -- 3 files changed, 2 insertions(+), 8 deletions(-) (limited to 'googletest') diff --git a/googletest/include/gtest/internal/gtest-internal.h b/googletest/include/gtest/internal/gtest-internal.h index 7f1a5b00..fabc8042 100644 --- a/googletest/include/gtest/internal/gtest-internal.h +++ b/googletest/include/gtest/internal/gtest-internal.h @@ -1120,8 +1120,6 @@ class NativeArray { const Element* array_; size_t size_; void (NativeArray::*clone_)(const Element*, size_t); - - GTEST_DISALLOW_ASSIGN_(NativeArray); }; // Backport of std::index_sequence. diff --git a/googletest/include/gtest/internal/gtest-port.h b/googletest/include/gtest/internal/gtest-port.h index 21fcf822..44307a13 100644 --- a/googletest/include/gtest/internal/gtest-port.h +++ b/googletest/include/gtest/internal/gtest-port.h @@ -682,7 +682,7 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_COPY_AND_ASSIGN_(type) \ type(type const &) = delete; \ - GTEST_DISALLOW_ASSIGN_(type) + type& operator=(type const &) = delete // A macro to disallow move operator= // This should be used in the private: declarations for a class. @@ -693,7 +693,7 @@ typedef struct _RTL_CRITICAL_SECTION GTEST_CRITICAL_SECTION; // This should be used in the private: declarations for a class. #define GTEST_DISALLOW_MOVE_AND_ASSIGN_(type) \ type(type &&) noexcept = delete; \ - GTEST_DISALLOW_MOVE_ASSIGN_(type) + type& operator=(type &&) noexcept = delete // Tell the compiler to warn about unused return values for functions declared // with this macro. The macro should be used on function declarations @@ -920,8 +920,6 @@ class GTEST_API_ RE { const char* full_pattern_; // For FullMatch(); # endif - - GTEST_DISALLOW_ASSIGN_(RE); }; #endif // GTEST_USES_PCRE diff --git a/googletest/test/googletest-port-test.cc b/googletest/test/googletest-port-test.cc index 60d637c3..2af75c20 100644 --- a/googletest/test/googletest-port-test.cc +++ b/googletest/test/googletest-port-test.cc @@ -1180,8 +1180,6 @@ class DestructorTracker { return DestructorCall::List().size() - 1; } const size_t index_; - - GTEST_DISALLOW_ASSIGN_(DestructorTracker); }; typedef ThreadLocal* ThreadParam; -- cgit v1.2.3 From 01c0ff5e23735ed6b2e71904cde69f57efeae499 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Thu, 16 Apr 2020 16:07:43 -0400 Subject: Fix a -Wdeprecated warning. googletest-port-test.cc:97:11: error: definition of implicit copy constructor for 'Base' is deprecated because it has a user-declared destructor [-Werror,-Wdeprecated] virtual ~Base() {} ^ --- googletest/test/googletest-port-test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'googletest') diff --git a/googletest/test/googletest-port-test.cc b/googletest/test/googletest-port-test.cc index 2af75c20..44b99ce5 100644 --- a/googletest/test/googletest-port-test.cc +++ b/googletest/test/googletest-port-test.cc @@ -90,10 +90,10 @@ TEST(IsXDigitTest, ReturnsFalseForWideNonAscii) { class Base { public: - // Copy constructor and assignment operator do exactly what we need, so we - // use them. Base() : member_(0) {} explicit Base(int n) : member_(n) {} + Base(const Base&) = default; + Base& operator=(const Base&) = default; virtual ~Base() {} int member() { return member_; } -- cgit v1.2.3 From c7d8ec72cc4baedbe1c3b1b304119e7a8b1a9262 Mon Sep 17 00:00:00 2001 From: Arthur O'Dwyer Date: Thu, 16 Apr 2020 16:15:10 -0400 Subject: Fix a -Wdeprecated warning. googletest-param-test-test.cc:502:8: error: definition of implicit copy constructor for 'NonDefaultConstructAssignString' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated] void operator=(const NonDefaultConstructAssignString&); ^ --- googletest/test/googletest-param-test-test.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'googletest') diff --git a/googletest/test/googletest-param-test-test.cc b/googletest/test/googletest-param-test-test.cc index 6ba89654..2b26e95f 100644 --- a/googletest/test/googletest-param-test-test.cc +++ b/googletest/test/googletest-param-test-test.cc @@ -490,16 +490,15 @@ TEST(CombineTest, CombineWithMaxNumberOfParameters) { class NonDefaultConstructAssignString { public: NonDefaultConstructAssignString(const std::string& s) : str_(s) {} + NonDefaultConstructAssignString() = delete; + NonDefaultConstructAssignString(const NonDefaultConstructAssignString&) = default; + NonDefaultConstructAssignString& operator=(const NonDefaultConstructAssignString&) = delete; + ~NonDefaultConstructAssignString() = default; const std::string& str() const { return str_; } private: std::string str_; - - // Not default constructible - NonDefaultConstructAssignString(); - // Not assignable - void operator=(const NonDefaultConstructAssignString&); }; TEST(CombineTest, NonDefaultConstructAssign) { -- cgit v1.2.3