Skip to content

Commit 7b109f4

Browse files
oldnewthingoldnewthing
authored
Prevent inadvertent assignment to temporary object (#825)
1 parent ac252b2 commit 7b109f4

File tree

10 files changed

+136
-22
lines changed

10 files changed

+136
-22
lines changed

‎cppwinrt/code_writers.h‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2319,6 +2319,10 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23192319
{
23202320
%(std::nullptr_t = nullptr) noexcept {}
23212321
%(void* ptr, take_ownership_from_abi_t) noexcept : Windows::Foundation::IInspectable(ptr, take_ownership_from_abi) {}
2322+
%(% const&) noexcept = default;
2323+
%(%&&) noexcept = default;
2324+
%& operator=(% const&) & noexcept = default;
2325+
%& operator=(%&&) & noexcept = default;
23222326
%% };
23232327
)";
23242328

@@ -2328,6 +2332,14 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23282332
bind<write_interface_requires>(type),
23292333
type_name,
23302334
type_name,
2335+
type_name, // %(T const&)
2336+
type_name, // T(% const&)
2337+
type_name, // %(T&&)
2338+
type_name, // T(%&&)
2339+
type_name, // %& operator=(T const&)
2340+
type_name, // T& operator=(% const&)
2341+
type_name, // %& operator=(T&&)
2342+
type_name, // T& operator=(%&&)
23312343
bind<write_interface_usings>(type),
23322344
bind<write_interface_extensions>(type));
23332345
}
@@ -2342,6 +2354,10 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23422354
{%
23432355
%(std::nullptr_t = nullptr) noexcept {}
23442356
%(void* ptr, take_ownership_from_abi_t) noexcept : Windows::Foundation::IInspectable(ptr, take_ownership_from_abi) {}
2357+
%(% const&) noexcept = default;
2358+
%(%&&) noexcept = default;
2359+
%& operator=(% const&) & noexcept = default;
2360+
%& operator=(%&&) & noexcept = default;
23452361
%% };
23462362
)";
23472363

@@ -2353,6 +2369,14 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23532369
bind<write_generic_asserts>(generics),
23542370
type_name,
23552371
type_name,
2372+
type_name, // %(T const&)
2373+
type_name, // T(% const&)
2374+
type_name, // %(T&&)
2375+
type_name, // T(%&&)
2376+
type_name, // %& operator=(T const&)
2377+
type_name, // T& operator=(% const&)
2378+
type_name, // %& operator=(T&&)
2379+
type_name, // T& operator=(%&&)
23562380
bind<write_interface_usings>(type),
23572381
bind<write_interface_extensions>(type));
23582382
}
@@ -2378,6 +2402,10 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23782402
{%
23792403
%(std::nullptr_t = nullptr) noexcept {}
23802404
%(void* ptr, take_ownership_from_abi_t) noexcept : Windows::Foundation::IUnknown(ptr, take_ownership_from_abi) {}
2405+
%(% const&) noexcept = default;
2406+
%(%&&) noexcept = default;
2407+
%& operator=(% const&) & noexcept = default;
2408+
%& operator=(%&&) & noexcept = default;
23812409
template <typename L> %(L lambda);
23822410
template <typename F> %(F* function);
23832411
template <typename O, typename M> %(O* object, M method);
@@ -2390,10 +2418,18 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
23902418
method_signature signature{ get_delegate_method(type) };
23912419

23922420
w.write(format,
2393-
type_name,
2421+
type_name, // struct %
23942422
bind<write_generic_asserts>(generics),
23952423
type_name,
23962424
type_name,
2425+
type_name, // %(T const&)
2426+
type_name, // T(% const&)
2427+
type_name, // %(T&&)
2428+
type_name, // T(%&&)
2429+
type_name, // %& operator=(T const&)
2430+
type_name, // T& operator=(% const&)
2431+
type_name, // %& operator=(T&&)
2432+
type_name, // T& operator=(%&&)
23972433
type_name,
23982434
type_name,
23992435
type_name,
@@ -3067,7 +3103,11 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
30673103
{
30683104
%(std::nullptr_t) noexcept {}
30693105
%(void* ptr, take_ownership_from_abi_t) noexcept : %(ptr, take_ownership_from_abi) {}
3070-
%%% };
3106+
% %(% const&) noexcept = default;
3107+
%(%&&) noexcept = default;
3108+
%& operator=(% const&) & noexcept = default;
3109+
%& operator=(%&&) & noexcept = default;
3110+
%% };
30713111
)";
30723112

30733113
w.write(format,
@@ -3079,6 +3119,14 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
30793119
type_name,
30803120
base_type,
30813121
bind<write_constructor_declarations>(type, factories),
3122+
type_name, // %(T const&)
3123+
type_name, // T(% const&)
3124+
type_name, // %(T%&)
3125+
type_name, // T(%&&)
3126+
type_name, // %& operator=(T const&)
3127+
type_name, // T& operator=(% const&)
3128+
type_name, // %& operator=(T&&)
3129+
type_name, // T& operator=(%&&)
30823130
bind<write_class_usings>(type),
30833131
bind_each<write_static_declaration>(factories, type));
30843132
}
@@ -3092,7 +3140,11 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
30923140
{
30933141
%(std::nullptr_t) noexcept {}
30943142
%(void* ptr, take_ownership_from_abi_t) noexcept : %(ptr, take_ownership_from_abi) {}
3095-
%%% };
3143+
% %(% const&) noexcept = default;
3144+
%(%&&) noexcept = default;
3145+
%& operator=(% const&) & noexcept = default;
3146+
%& operator=(%&&) & noexcept = default;
3147+
%% };
30963148
)";
30973149

30983150
w.write(format,
@@ -3103,6 +3155,14 @@ struct __declspec(empty_bases) produce_dispatch_to_overridable<T, D, %>
31033155
type_name,
31043156
base_type,
31053157
bind<write_constructor_declarations>(type, factories),
3158+
type_name, // %(T const&)
3159+
type_name, // T(% const&)
3160+
type_name, // %(T%&)
3161+
type_name, // T(%&&)
3162+
type_name, // %& operator=(T const&)
3163+
type_name, // T& operator=(% const&)
3164+
type_name, // %& operator=(T&&)
3165+
type_name, // T& operator=(%&&)
31063166
bind<write_fast_class_base_declarations>(type),
31073167
bind_each<write_static_declaration>(factories, type));
31083168
}

‎strings/base_activation.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,10 @@ WINRT_EXPORT namespace winrt
516516
{
517517
IActivationFactory(std::nullptr_t = nullptr) noexcept {}
518518
IActivationFactory(void* ptr, take_ownership_from_abi_t) noexcept : IInspectable(ptr, take_ownership_from_abi) {}
519+
IActivationFactory(IActivationFactory const&) noexcept = default;
520+
IActivationFactory(IActivationFactory&&) noexcept = default;
521+
IActivationFactory& operator=(IActivationFactory const&) & noexcept = default;
522+
IActivationFactory& operator=(IActivationFactory&&) & noexcept = default;
519523

520524
template <typename T>
521525
T ActivateInstance() const

‎strings/base_array.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ WINRT_EXPORT namespace winrt
295295
other.m_size = 0;
296296
}
297297

298-
com_array& operator=(com_array&& other) noexcept
298+
com_array& operator=(com_array&& other) & noexcept
299299
{
300300
clear();
301301
this->m_data = other.m_data;

‎strings/base_com_ptr.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ WINRT_EXPORT namespace winrt
3333
release_ref();
3434
}
3535

36-
com_ptr& operator=(com_ptr const& other) noexcept
36+
com_ptr& operator=(com_ptr const& other) & noexcept
3737
{
3838
copy_ref(other.m_ptr);
3939
return*this;
4040
}
4141

42-
com_ptr& operator=(com_ptr&& other) noexcept
42+
com_ptr& operator=(com_ptr&& other) & noexcept
4343
{
4444
if (this != &other)
4545
{
@@ -51,14 +51,14 @@ WINRT_EXPORT namespace winrt
5151
}
5252

5353
template <typename U>
54-
com_ptr& operator=(com_ptr<U> const& other) noexcept
54+
com_ptr& operator=(com_ptr<U> const& other) & noexcept
5555
{
5656
copy_ref(other.m_ptr);
5757
return*this;
5858
}
5959

6060
template <typename U>
61-
com_ptr& operator=(com_ptr<U>&& other) noexcept
61+
com_ptr& operator=(com_ptr<U>&& other) & noexcept
6262
{
6363
release_ref();
6464
m_ptr = std::exchange(other.m_ptr, {});

‎strings/base_events.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ WINRT_EXPORT namespace winrt
2929
event_revoker& operator=(event_revoker const&) = delete;
3030
event_revoker(event_revoker&&) noexcept = default;
3131

32-
event_revoker& operator=(event_revoker&& other) noexcept
32+
event_revoker& operator=(event_revoker&& other) & noexcept
3333
{
3434
if (this != &other)
3535
{
@@ -84,7 +84,7 @@ WINRT_EXPORT namespace winrt
8484
factory_event_revoker& operator=(factory_event_revoker const&) = delete;
8585
factory_event_revoker(factory_event_revoker&&) noexcept = default;
8686

87-
factory_event_revoker& operator=(factory_event_revoker&& other) noexcept
87+
factory_event_revoker& operator=(factory_event_revoker&& other) & noexcept
8888
{
8989
if (this != &other)
9090
{
@@ -140,7 +140,7 @@ namespace winrt::impl
140140
event_revoker& operator=(event_revoker const&) = delete;
141141

142142
event_revoker(event_revoker&&) noexcept = default;
143-
event_revoker& operator=(event_revoker&& other) noexcept
143+
event_revoker& operator=(event_revoker&& other) & noexcept
144144
{
145145
event_revoker(std::move(other)).swap(*this);
146146
return *this;

‎strings/base_handle.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ WINRT_EXPORT namespace winrt
1616
{
1717
}
1818

19-
handle_type& operator=(handle_type&& other) noexcept
19+
handle_type& operator=(handle_type&& other) & noexcept
2020
{
2121
if (this != &other)
2222
{

‎strings/base_security.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ WINRT_EXPORT namespace winrt
5555

5656
access_token() = default;
5757
access_token(access_token&& other) = default;
58-
access_token& operator=(access_token&& other) = default;
58+
access_token& operator=(access_token&& other) & = default;
5959

6060
access_token impersonate() const
6161
{

‎strings/base_string.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,16 @@ WINRT_EXPORT namespace winrt
179179
m_handle(impl::duplicate_hstring(value.m_handle.get()))
180180
{}
181181

182-
hstring& operator=(hstring const& value)
182+
hstring& operator=(hstring const& value) &
183183
{
184184
m_handle.attach(impl::duplicate_hstring(value.m_handle.get()));
185185
return*this;
186186
}
187187

188188
hstring(hstring&&) noexcept = default;
189-
hstring& operator=(hstring&&) = default;
189+
hstring& operator=(hstring&&) & = default;
190190
hstring(std::nullptr_t) = delete;
191-
hstring& operator=(std::nullptr_t) = delete;
191+
hstring& operator=(std::nullptr_t) & = delete;
192192

193193
hstring(std::initializer_list<wchar_t> value) :
194194
hstring(value.begin(), static_cast<uint32_t>(value.size()))
@@ -206,17 +206,17 @@ WINRT_EXPORT namespace winrt
206206
hstring(value.data(), static_cast<size_type>(value.size()))
207207
{}
208208

209-
hstring& operator=(std::wstring_view const& value)
209+
hstring& operator=(std::wstring_view const& value) &
210210
{
211211
return *this = hstring{ value };
212212
}
213213

214-
hstring& operator=(wchar_t const* const value)
214+
hstring& operator=(wchar_t const* const value) &
215215
{
216216
return *this = hstring{ value };
217217
}
218218

219-
hstring& operator=(std::initializer_list<wchar_t> value)
219+
hstring& operator=(std::initializer_list<wchar_t> value) &
220220
{
221221
return *this = hstring{ value };
222222
}

‎strings/base_windows.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
161161
release_ref();
162162
}
163163

164-
IUnknown& operator=(IUnknown const& other) noexcept
164+
IUnknown& operator=(IUnknown const& other) & noexcept
165165
{
166166
if (this != &other)
167167
{
@@ -173,7 +173,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
173173
return*this;
174174
}
175175

176-
IUnknown& operator=(IUnknown&& other) noexcept
176+
IUnknown& operator=(IUnknown&& other) & noexcept
177177
{
178178
if (this != &other)
179179
{
@@ -189,7 +189,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
189189
return nullptr != m_ptr;
190190
}
191191

192-
IUnknown& operator=(std::nullptr_t) noexcept
192+
IUnknown& operator=(std::nullptr_t) & noexcept
193193
{
194194
release_ref();
195195
return*this;
@@ -425,5 +425,9 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
425425
{
426426
IInspectable(std::nullptr_t = nullptr) noexcept {}
427427
IInspectable(void* ptr, take_ownership_from_abi_t) noexcept : IUnknown(ptr, take_ownership_from_abi) {}
428+
IInspectable(IInspectable const&) noexcept = default;
429+
IInspectable(IInspectable&&) noexcept = default;
430+
IInspectable& operator=(IInspectable const&) & noexcept = default;
431+
IInspectable& operator=(IInspectable&&) & noexcept = default;
428432
};
429433
}

‎test/old_tests/UnitTests/properties.cpp‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,50 @@ TEST_CASE("properties")
4444

4545
REQUIRE_THROWS_AS(e.Name(L"throw"), hresult_invalid_argument);
4646
REQUIRE_THROWS_AS(e.Name(), hresult_invalid_argument);
47+
48+
}
49+
50+
namespace
51+
{
52+
// Make sure that statements like
53+
//
54+
// e.Name() = L"Fred"; // intended e.Name(L"Fred");
55+
// e.Uri() = newUri; // intended e.Uri(newUri);
56+
//
57+
// are not valid. These are common beginner errors when
58+
// trying to set Windows Runtime properties.
59+
60+
template<typename T, bool default_constructible, typename... ConstructorArgs>
61+
struct validate_rvalue_operations
62+
{
63+
// Make sure we didn't damage default constructor.
64+
static_assert(std::is_default_constructible_v<T> == default_constructible);
65+
66+
// Make sure we didn't damage other constructors.
67+
static_assert((std::is_constructible_v<T, ConstructorArgs> && ...));
68+
69+
// Make sure we didn't damage copy and move constructors.
70+
static_assert(std::is_copy_constructible_v<T>);
71+
static_assert(std::is_move_constructible_v<T>);
72+
73+
// Make sure rvalue assignment is disallowed, but lvalue is still okay.
74+
static_assert(!std::is_assignable_v<T&&, T>);
75+
static_assert((!std::is_assignable_v<T&&, ConstructorArgs> && ...));
76+
static_assert(std::is_assignable_v<T&, T>);
77+
static_assert((std::is_assignable_v<T&, ConstructorArgs> && ...));
78+
79+
constexpr static bool validate()
80+
{
81+
// Dummy method. Exists only to force instantiation of type so the static_assert's will fire.
82+
return true;
83+
}
84+
};
85+
86+
static_assert(validate_rvalue_operations<hstring, true, const wchar_t*>::validate());
87+
static_assert(validate_rvalue_operations<Windows::Foundation::IInspectable, true, std::nullptr_t>::validate());
88+
static_assert(validate_rvalue_operations<Windows::Foundation::IActivationFactory, true, std::nullptr_t>::validate());
89+
static_assert(validate_rvalue_operations<Windows::Foundation::IAsyncOperation<int32_t>, true, std::nullptr_t>::validate());
90+
static_assert(validate_rvalue_operations<Windows::Foundation::EventHandler<Windows::Foundation::IInspectable>, true, std::nullptr_t>::validate());
91+
static_assert(validate_rvalue_operations<Windows::Foundation::IUriRuntimeClass, true, std::nullptr_t>::validate());
92+
static_assert(validate_rvalue_operations<Windows::Foundation::Uri, false /* not default constructible */, std::nullptr_t>::validate());
4793
}

0 commit comments

Comments
 (0)