From 7e730e7652a42754bd0384feb7c84d5484daff2e Mon Sep 17 00:00:00 2001 From: Kenny Kerr Date: Wed, 7 Jul 2021 07:14:04 -0700 Subject: [PATCH] Revert "Prevent inadvertent assignment to temporary object" (#976) --- cppwinrt/code_writers.h | 66 ++----------------------- strings/base_activation.h | 4 -- strings/base_array.h | 2 +- strings/base_com_ptr.h | 8 +-- strings/base_events.h | 6 +-- strings/base_handle.h | 2 +- strings/base_security.h | 2 +- strings/base_string.h | 12 ++--- strings/base_windows.h | 10 ++-- test/old_tests/UnitTests/properties.cpp | 46 ----------------- 10 files changed, 22 insertions(+), 136 deletions(-) diff --git a/cppwinrt/code_writers.h b/cppwinrt/code_writers.h index 95a2fa59e..df4d66564 100644 --- a/cppwinrt/code_writers.h +++ b/cppwinrt/code_writers.h @@ -2371,10 +2371,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable { %(std::nullptr_t = nullptr) noexcept {} %(void* ptr, take_ownership_from_abi_t) noexcept : winrt::Windows::Foundation::IInspectable(ptr, take_ownership_from_abi) {} - %(% const&) noexcept = default; - %(%&&) noexcept = default; - %& operator=(% const&) & noexcept = default; - %& operator=(%&&) & noexcept = default; %% }; )"; @@ -2384,14 +2380,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable bind(type), type_name, type_name, - type_name, // %(T const&) - type_name, // T(% const&) - type_name, // %(T&&) - type_name, // T(%&&) - type_name, // %& operator=(T const&) - type_name, // T& operator=(% const&) - type_name, // %& operator=(T&&) - type_name, // T& operator=(%&&) bind(type), bind(type)); } @@ -2406,10 +2394,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable {% %(std::nullptr_t = nullptr) noexcept {} %(void* ptr, take_ownership_from_abi_t) noexcept : winrt::Windows::Foundation::IInspectable(ptr, take_ownership_from_abi) {} - %(% const&) noexcept = default; - %(%&&) noexcept = default; - %& operator=(% const&) & noexcept = default; - %& operator=(%&&) & noexcept = default; %% }; )"; @@ -2421,14 +2405,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable bind(generics), type_name, type_name, - type_name, // %(T const&) - type_name, // T(% const&) - type_name, // %(T&&) - type_name, // T(%&&) - type_name, // %& operator=(T const&) - type_name, // T& operator=(% const&) - type_name, // %& operator=(T&&) - type_name, // T& operator=(%&&) bind(type), bind(type)); } @@ -2454,10 +2430,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable {% %(std::nullptr_t = nullptr) noexcept {} %(void* ptr, take_ownership_from_abi_t) noexcept : Windows::Foundation::IUnknown(ptr, take_ownership_from_abi) {} - %(% const&) noexcept = default; - %(%&&) noexcept = default; - %& operator=(% const&) & noexcept = default; - %& operator=(%&&) & noexcept = default; template %(L lambda); template %(F* function); template %(O* object, M method); @@ -2470,18 +2442,10 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable method_signature signature{ get_delegate_method(type) }; w.write(format, - type_name, // struct % + type_name, bind(generics), type_name, type_name, - type_name, // %(T const&) - type_name, // T(% const&) - type_name, // %(T&&) - type_name, // T(%&&) - type_name, // %& operator=(T const&) - type_name, // T& operator=(% const&) - type_name, // %& operator=(T&&) - type_name, // T& operator=(%&&) type_name, type_name, type_name, @@ -3155,11 +3119,7 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable { %(std::nullptr_t) noexcept {} %(void* ptr, take_ownership_from_abi_t) noexcept : %(ptr, take_ownership_from_abi) {} -% %(% const&) noexcept = default; - %(%&&) noexcept = default; - %& operator=(% const&) & noexcept = default; - %& operator=(%&&) & noexcept = default; -%% }; +%%% }; )"; w.write(format, @@ -3171,14 +3131,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable type_name, base_type, bind(type, factories), - type_name, // %(T const&) - type_name, // T(% const&) - type_name, // %(T%&) - type_name, // T(%&&) - type_name, // %& operator=(T const&) - type_name, // T& operator=(% const&) - type_name, // %& operator=(T&&) - type_name, // T& operator=(%&&) bind(type), bind_each(factories, type)); } @@ -3192,11 +3144,7 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable { %(std::nullptr_t) noexcept {} %(void* ptr, take_ownership_from_abi_t) noexcept : %(ptr, take_ownership_from_abi) {} -% %(% const&) noexcept = default; - %(%&&) noexcept = default; - %& operator=(% const&) & noexcept = default; - %& operator=(%&&) & noexcept = default; -%% }; +%%% }; )"; w.write(format, @@ -3207,14 +3155,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable type_name, base_type, bind(type, factories), - type_name, // %(T const&) - type_name, // T(% const&) - type_name, // %(T%&) - type_name, // T(%&&) - type_name, // %& operator=(T const&) - type_name, // T& operator=(% const&) - type_name, // %& operator=(T&&) - type_name, // T& operator=(%&&) bind(type), bind_each(factories, type)); } diff --git a/strings/base_activation.h b/strings/base_activation.h index a54c9e23b..942b3b342 100644 --- a/strings/base_activation.h +++ b/strings/base_activation.h @@ -523,10 +523,6 @@ WINRT_EXPORT namespace winrt { IActivationFactory(std::nullptr_t = nullptr) noexcept {} IActivationFactory(void* ptr, take_ownership_from_abi_t) noexcept : IInspectable(ptr, take_ownership_from_abi) {} - IActivationFactory(IActivationFactory const&) noexcept = default; - IActivationFactory(IActivationFactory&&) noexcept = default; - IActivationFactory& operator=(IActivationFactory const&) & noexcept = default; - IActivationFactory& operator=(IActivationFactory&&) & noexcept = default; template T ActivateInstance() const diff --git a/strings/base_array.h b/strings/base_array.h index 06e53a5b8..5f0904efe 100644 --- a/strings/base_array.h +++ b/strings/base_array.h @@ -295,7 +295,7 @@ WINRT_EXPORT namespace winrt other.m_size = 0; } - com_array& operator=(com_array&& other) & noexcept + com_array& operator=(com_array&& other) noexcept { clear(); this->m_data = other.m_data; diff --git a/strings/base_com_ptr.h b/strings/base_com_ptr.h index ed6bf0089..1c7536e0f 100644 --- a/strings/base_com_ptr.h +++ b/strings/base_com_ptr.h @@ -57,13 +57,13 @@ WINRT_EXPORT namespace winrt release_ref(); } - com_ptr& operator=(com_ptr const& other) & noexcept + com_ptr& operator=(com_ptr const& other) noexcept { copy_ref(other.m_ptr); return*this; } - com_ptr& operator=(com_ptr&& other) & noexcept + com_ptr& operator=(com_ptr&& other) noexcept { if (this != &other) { @@ -75,14 +75,14 @@ WINRT_EXPORT namespace winrt } template - com_ptr& operator=(com_ptr const& other) & noexcept + com_ptr& operator=(com_ptr const& other) noexcept { copy_ref(other.m_ptr); return*this; } template - com_ptr& operator=(com_ptr&& other) & noexcept + com_ptr& operator=(com_ptr&& other) noexcept { release_ref(); m_ptr = std::exchange(other.m_ptr, {}); diff --git a/strings/base_events.h b/strings/base_events.h index 3b53fa06f..8b4edb060 100644 --- a/strings/base_events.h +++ b/strings/base_events.h @@ -29,7 +29,7 @@ WINRT_EXPORT namespace winrt event_revoker& operator=(event_revoker const&) = delete; event_revoker(event_revoker&&) noexcept = default; - event_revoker& operator=(event_revoker&& other) & noexcept + event_revoker& operator=(event_revoker&& other) noexcept { if (this != &other) { @@ -84,7 +84,7 @@ WINRT_EXPORT namespace winrt factory_event_revoker& operator=(factory_event_revoker const&) = delete; factory_event_revoker(factory_event_revoker&&) noexcept = default; - factory_event_revoker& operator=(factory_event_revoker&& other) & noexcept + factory_event_revoker& operator=(factory_event_revoker&& other) noexcept { if (this != &other) { @@ -140,7 +140,7 @@ namespace winrt::impl event_revoker& operator=(event_revoker const&) = delete; event_revoker(event_revoker&&) noexcept = default; - event_revoker& operator=(event_revoker&& other) & noexcept + event_revoker& operator=(event_revoker&& other) noexcept { event_revoker(std::move(other)).swap(*this); return *this; diff --git a/strings/base_handle.h b/strings/base_handle.h index 1cd5b4d82..60eec8be0 100644 --- a/strings/base_handle.h +++ b/strings/base_handle.h @@ -16,7 +16,7 @@ WINRT_EXPORT namespace winrt { } - handle_type& operator=(handle_type&& other) & noexcept + handle_type& operator=(handle_type&& other) noexcept { if (this != &other) { diff --git a/strings/base_security.h b/strings/base_security.h index 18c6f7d61..8160f11ad 100644 --- a/strings/base_security.h +++ b/strings/base_security.h @@ -55,7 +55,7 @@ WINRT_EXPORT namespace winrt access_token() = default; access_token(access_token&& other) = default; - access_token& operator=(access_token&& other) & = default; + access_token& operator=(access_token&& other) = default; access_token impersonate() const { diff --git a/strings/base_string.h b/strings/base_string.h index b9fada8d2..f50403c41 100644 --- a/strings/base_string.h +++ b/strings/base_string.h @@ -179,16 +179,16 @@ WINRT_EXPORT namespace winrt m_handle(impl::duplicate_hstring(value.m_handle.get())) {} - hstring& operator=(hstring const& value) & + hstring& operator=(hstring const& value) { m_handle.attach(impl::duplicate_hstring(value.m_handle.get())); return*this; } hstring(hstring&&) noexcept = default; - hstring& operator=(hstring&&) & = default; + hstring& operator=(hstring&&) = default; hstring(std::nullptr_t) = delete; - hstring& operator=(std::nullptr_t) & = delete; + hstring& operator=(std::nullptr_t) = delete; hstring(std::initializer_list value) : hstring(value.begin(), static_cast(value.size())) @@ -206,17 +206,17 @@ WINRT_EXPORT namespace winrt hstring(value.data(), static_cast(value.size())) {} - hstring& operator=(std::wstring_view const& value) & + hstring& operator=(std::wstring_view const& value) { return *this = hstring{ value }; } - hstring& operator=(wchar_t const* const value) & + hstring& operator=(wchar_t const* const value) { return *this = hstring{ value }; } - hstring& operator=(std::initializer_list value) & + hstring& operator=(std::initializer_list value) { return *this = hstring{ value }; } diff --git a/strings/base_windows.h b/strings/base_windows.h index 6da4c394a..3800af1b1 100644 --- a/strings/base_windows.h +++ b/strings/base_windows.h @@ -161,7 +161,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation release_ref(); } - IUnknown& operator=(IUnknown const& other) & noexcept + IUnknown& operator=(IUnknown const& other) noexcept { if (this != &other) { @@ -173,7 +173,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation return*this; } - IUnknown& operator=(IUnknown&& other) & noexcept + IUnknown& operator=(IUnknown&& other) noexcept { if (this != &other) { @@ -189,7 +189,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation return nullptr != m_ptr; } - IUnknown& operator=(std::nullptr_t) & noexcept + IUnknown& operator=(std::nullptr_t) noexcept { release_ref(); return*this; @@ -425,9 +425,5 @@ WINRT_EXPORT namespace winrt::Windows::Foundation { IInspectable(std::nullptr_t = nullptr) noexcept {} IInspectable(void* ptr, take_ownership_from_abi_t) noexcept : IUnknown(ptr, take_ownership_from_abi) {} - IInspectable(IInspectable const&) noexcept = default; - IInspectable(IInspectable&&) noexcept = default; - IInspectable& operator=(IInspectable const&) & noexcept = default; - IInspectable& operator=(IInspectable&&) & noexcept = default; }; } diff --git a/test/old_tests/UnitTests/properties.cpp b/test/old_tests/UnitTests/properties.cpp index da219fbb8..ff17a0324 100644 --- a/test/old_tests/UnitTests/properties.cpp +++ b/test/old_tests/UnitTests/properties.cpp @@ -44,50 +44,4 @@ TEST_CASE("properties") REQUIRE_THROWS_AS(e.Name(L"throw"), hresult_invalid_argument); REQUIRE_THROWS_AS(e.Name(), hresult_invalid_argument); - -} - -namespace -{ - // Make sure that statements like - // - // e.Name() = L"Fred"; // intended e.Name(L"Fred"); - // e.Uri() = newUri; // intended e.Uri(newUri); - // - // are not valid. These are common beginner errors when - // trying to set Windows Runtime properties. - - template - struct validate_rvalue_operations - { - // Make sure we didn't damage default constructor. - static_assert(std::is_default_constructible_v == default_constructible); - - // Make sure we didn't damage other constructors. - static_assert((std::is_constructible_v && ...)); - - // Make sure we didn't damage copy and move constructors. - static_assert(std::is_copy_constructible_v); - static_assert(std::is_move_constructible_v); - - // Make sure rvalue assignment is disallowed, but lvalue is still okay. - static_assert(!std::is_assignable_v); - static_assert((!std::is_assignable_v && ...)); - static_assert(std::is_assignable_v); - static_assert((std::is_assignable_v && ...)); - - constexpr static bool validate() - { - // Dummy method. Exists only to force instantiation of type so the static_assert's will fire. - return true; - } - }; - - static_assert(validate_rvalue_operations::validate()); - static_assert(validate_rvalue_operations::validate()); - static_assert(validate_rvalue_operations::validate()); - static_assert(validate_rvalue_operations, true, std::nullptr_t>::validate()); - static_assert(validate_rvalue_operations, true, std::nullptr_t>::validate()); - static_assert(validate_rvalue_operations::validate()); - static_assert(validate_rvalue_operations::validate()); }