Skip to content

Prevent inadvertent assignment to temporary object - #825

Merged
Kenny Kerr (kennykerr) merged 1 commit into
microsoft:masterfrom
oldnewthing:assignment-to-temporary
Dec 17, 2020
Merged

Prevent inadvertent assignment to temporary object#825
Kenny Kerr (kennykerr) merged 1 commit into
microsoft:masterfrom
oldnewthing:assignment-to-temporary

Conversation

@oldnewthing

Copy link
Copy Markdown
Member

Common porting error coming from languages that have properties as native concepts is trying to set a C++/WinRT property by doing

o.Property() = value;

due to overzealous application of the rule "In C++/WinRT, you access a property by doing o.Property()."

Fix this by making all assignment operators require the assigned-to variable to be an lvalue, rendering the above mistake a compile-time error.

Assigning to an rvalue is not interesting because the rvalue has no name, so you have no way of using the assigned-to object. There are some wacky fringe cases where you might be doing this on purpose:

get_abi(hstring() = L"Hello")

gives you an ABI string handle for a temporary. But this is decidedly non-idiomatic and I'm fairly confident nobody does this on purpose. The idiomatic way of writing this is simply

get_abi(hstring(L"Hello"))

This fixes issue #359

Implementation notes

Assignment operators default to non-ref-qualified, which makes them applicable to both lvalue and rvalue references. We must explicitly redeclare them as lvalue ref-qualified in order to remove the rvalue ref-qualified version.

Once you have an explicit assignment operator, the implicit copy and move constructors disappear, so we have to bring them back explicitly.

Once you add explicit copy and move constructors, then the implicit default constructor disappears. Fortunately, we never relied upon the implicit default constructor, so we don't have to to make explicit versions.

Common porting error coming from languages that have
properties is trying to set a C++/WinRT property by doing

```cpp
o.Property() = value;
```

due to overzealous application of the rule "In C++/WinRT, you
access a property by doing `o.Property()`."

Fix this by making all assignment operators require the
assigned-to variable to be an lvalue, rendering the above
mistake a compile-time error.

Assigning to an rvalue is not interesting because the rvalue
has no name, so you have no way of using the assigned-to
object. There are some wacky fringe cases where you might be
doing this on purpose:

```cpp
get_abi(hstring() = L"Hello")
```

gives you an ABI string handle for a temporary. But this is
decidedly non-idiomatic and I think it's okay to break them.
The idiomatic way of writing this is simply

```cpp
get_abi(hstring(L"Hello"))
```

Assignment operators default to non-ref-qualified, which makes
them applicable to both lvalue and rvalue references. We must
explicitly redeclare them as lvalue ref-qualified in order to
remove the rvalue ref-qualified version.

Once you have an explicit assignment operator, the implicit copy
and move constructors disappear, so we have to bring them back
explicitly.

Once you add explicit copy and move constructors, then
the implicit default constructor disappears. Fortunately,
we never relied upon the implicit default constructor,
so we don't have to to make explicit versions.

@kennykerr Kenny Kerr (kennykerr) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kennykerr

Copy link
Copy Markdown
Collaborator

Ryan Shepherd (@DefaultRyan) see any issues with this? It looks good to me.

@DefaultRyan

Copy link
Copy Markdown
Member

Ryan Shepherd (@DefaultRyan) see any issues with this? It looks good to me.

Looks good to me. Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants