Skip to content

Commit

Permalink
Revert "Prevent inadvertent assignment to temporary object" (#976)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kennykerr
kennykerr authored Jul 7, 2021
1 parent 634d627 commit 7e730e7
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 136 deletions.
66 changes: 3 additions & 63 deletions cppwinrt/code_writers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2371,10 +2371,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
{
%(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;
%% };
)";

Expand All @@ -2384,14 +2380,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
bind<write_interface_requires>(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<write_interface_usings>(type),
bind<write_interface_extensions>(type));
}
Expand All @@ -2406,10 +2394,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
{%
%(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;
%% };
)";

Expand All @@ -2421,14 +2405,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
bind<write_generic_asserts>(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<write_interface_usings>(type),
bind<write_interface_extensions>(type));
}
Expand All @@ -2454,10 +2430,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
{%
%(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 <typename L> %(L lambda);
template <typename F> %(F* function);
template <typename O, typename M> %(O* object, M method);
Expand All @@ -2470,18 +2442,10 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
method_signature signature{ get_delegate_method(type) };

w.write(format,
type_name, // struct %
type_name,
bind<write_generic_asserts>(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,
Expand Down Expand Up @@ -3155,11 +3119,7 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
{
%(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,
Expand All @@ -3171,14 +3131,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
type_name,
base_type,
bind<write_constructor_declarations>(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<write_class_usings>(type),
bind_each<write_static_declaration>(factories, type));
}
Expand All @@ -3192,11 +3144,7 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
{
%(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,
Expand All @@ -3207,14 +3155,6 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
type_name,
base_type,
bind<write_constructor_declarations>(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<write_fast_class_base_declarations>(type),
bind_each<write_static_declaration>(factories, type));
}
Expand Down
4 changes: 0 additions & 4 deletions strings/base_activation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
T ActivateInstance() const
Expand Down
2 changes: 1 addition & 1 deletion strings/base_array.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions strings/base_com_ptr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -75,14 +75,14 @@ WINRT_EXPORT namespace winrt
}

template <typename U>
com_ptr& operator=(com_ptr<U> const& other) & noexcept
com_ptr& operator=(com_ptr<U> const& other) noexcept
{
copy_ref(other.m_ptr);
return*this;
}

template <typename U>
com_ptr& operator=(com_ptr<U>&& other) & noexcept
com_ptr& operator=(com_ptr<U>&& other) noexcept
{
release_ref();
m_ptr = std::exchange(other.m_ptr, {});
Expand Down
6 changes: 3 additions & 3 deletions strings/base_events.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion strings/base_handle.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
2 changes: 1 addition & 1 deletion strings/base_security.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
12 changes: 6 additions & 6 deletions strings/base_string.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<wchar_t> value) :
hstring(value.begin(), static_cast<uint32_t>(value.size()))
Expand All @@ -206,17 +206,17 @@ WINRT_EXPORT namespace winrt
hstring(value.data(), static_cast<size_type>(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<wchar_t> value) &
hstring& operator=(std::initializer_list<wchar_t> value)
{
return *this = hstring{ value };
}
Expand Down
10 changes: 3 additions & 7 deletions strings/base_windows.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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;
};
}
46 changes: 0 additions & 46 deletions test/old_tests/UnitTests/properties.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<typename T, bool default_constructible, typename... ConstructorArgs>
struct validate_rvalue_operations
{
// Make sure we didn't damage default constructor.
static_assert(std::is_default_constructible_v<T> == default_constructible);

// Make sure we didn't damage other constructors.
static_assert((std::is_constructible_v<T, ConstructorArgs> && ...));

// Make sure we didn't damage copy and move constructors.
static_assert(std::is_copy_constructible_v<T>);
static_assert(std::is_move_constructible_v<T>);

// Make sure rvalue assignment is disallowed, but lvalue is still okay.
static_assert(!std::is_assignable_v<T&&, T>);
static_assert((!std::is_assignable_v<T&&, ConstructorArgs> && ...));
static_assert(std::is_assignable_v<T&, T>);
static_assert((std::is_assignable_v<T&, ConstructorArgs> && ...));

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<hstring, true, const wchar_t*>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::IInspectable, true, std::nullptr_t>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::IActivationFactory, true, std::nullptr_t>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::IAsyncOperation<int32_t>, true, std::nullptr_t>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::EventHandler<Windows::Foundation::IInspectable>, true, std::nullptr_t>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::IUriRuntimeClass, true, std::nullptr_t>::validate());
static_assert(validate_rvalue_operations<Windows::Foundation::Uri, false /* not default constructible */, std::nullptr_t>::validate());
}

0 comments on commit 7e730e7

Please sign in to comment.