Remove paragraph on return std::move with implicit conversion [website : main]
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
Martin Bidlingmaier has uploaded this change for review.
Remove paragraph on return std::move with implicit conversion
Change-Id: I17d34b011eedd906552a6bf8583a021e9071928a
---
M site/rvalue-references/index.md
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/site/rvalue-references/index.md b/site/rvalue-references/index.md
index 8a7edb7..a9609e3 100644
--- a/site/rvalue-references/index.md
+++ b/site/rvalue-references/index.md
@@ -456,23 +456,6 @@
a previous chromium-dev thread about this in the context of scoped_ptr:
[link](https://groups.google.com/a/chromium.org/d/msg/chromium-dev/ICA3hiR4Z3A/Bi_ec46cAQAJ).
-However, if the types of the variable and the return type do not match exactly,
-then you will have to use std::move() in order to convert without creating an
-extra temporary.
-
-std::unique_ptr<MyType> MakeMyType()
-
-{
-
-std::unique_ptr<ChildClassOfMyType> ptr;
-
-// This call to std::move() is needed to convert from a pointer-to-the
-
-// child class to a pointer-to-the parent.
-
-return std::move(ptr);
-}
-
Similarly do not use move() on the return value from calling a function. You
already have an rvalue, so casting it there adds no value, only noise.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
Patch set 1:Commit-Queue +1
1 comment:
Patchset:
Hi everybody,
Can you have a look at this change about our c++ style with regards to move semantics? It's about situations such as this one:
```
struct A{};
struct B : A {};
std::unique_ptr<A> foo() {
auto result = std::make_unique<B>();
// Implicitly convert rvalue of unique_ptr<B> to unique_ptr<A>.
return std::move(result);
}
```As far as I know, compilers must treat a variable as rvalue when it is returned if it is local or a function parameter. Thus the `std::move` here is superfluous. Indeed, both clang and gcc compile the above snippet without the `std::move`.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Nico Weber (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
Patch set 1:Code-Review +1
1 comment:
File site/rvalue-references/index.md:
Patch Set #1, Line 467:
std::unique_ptr<ChildClassOfMyType> ptr;I think the point here was that the function return ChildClassOfMyType from a function returning MyType.
Before C++14 (?), this did need the explicit move, and I think this is a remnant from then. It's true that this is now no longer needed.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Elly Fong-Jones.
Martin Bidlingmaier would like Elly Fong-Jones to review this change.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Elly Fong-Jones.
Martin Bidlingmaier uploaded patch set #2 to this change.
Remove paragraph on return std::move with implicit conversion
The std::move is no longer needed in these situations.
Change-Id: I17d34b011eedd906552a6bf8583a021e9071928a
---
M site/rvalue-references/index.md
1 file changed, 11 insertions(+), 17 deletions(-)
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Elly Fong-Jones.
1 comment:
File site/rvalue-references/index.md:
Patch Set #1, Line 467:
std::unique_ptr<ChildClassOfMyType> ptr;
I think the point here was that the function return ChildClassOfMyType from a function returning MyT […]
Ack
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Elly Fong-Jones, Martin Bidlingmaier.
Martin Bidlingmaier uploaded patch set #5 to this change.
The following approvals got outdated and were removed: Commit-Queue+1 by Martin Bidlingmaier
Remove paragraph on return std::move with implicit conversion
The std::move is no longer needed in these situations.
Drive-by: Replace unique_ptr<...>(new ...) with make_unique in code snippet.
Change-Id: I17d34b011eedd906552a6bf8583a021e9071928a
---
M site/developers/smart-pointer-guidelines/index.md
M site/rvalue-references/index.md
2 files changed, 16 insertions(+), 24 deletions(-)
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Attention is currently required from: Elly Fong-Jones, Martin Bidlingmaier.
Martin Bidlingmaier uploaded patch set #6 to this change.
Remove paragraph on return std::move with implicit conversion
The std::move is no longer needed in these situations.
Drive-by: Replace unique_ptr<...>(new ...) with make_unique in code snippet.
Bug: b/240531273
Change-Id: I17d34b011eedd906552a6bf8583a021e9071928a
---
M site/developers/smart-pointer-guidelines/index.md
M site/rvalue-references/index.md
2 files changed, 17 insertions(+), 24 deletions(-)
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Elly Fong-Jones (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
Patch set 6:Code-Review +1
1 comment:
Patchset:
lgtm!
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
K. Moon (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
1 comment:
Patchset:
It's subtle, and I had to double-check this, but we need to distinguish between three cases here:
1. Does "return" perform a move? Since C++11, this is automatic for both local variables _and_ parameters:
https://en.cppreference.com/w/cpp/language/return#Automatic_move_from_local_variables_and_parameters2. Is a copy/move from a temporary optimized out? Since C++17, this "Return Value Optimization" is mandatory, as opposed to optional:
https://en.cppreference.com/w/cpp/language/copy_elision#Mandatory_elision_of_copy.2Fmove_operations3. Is a copy/move from an automatic variable optimized out? This is "Named RVO", and is _not_ mandatory (but a common optimization):
https://en.cppreference.com/w/cpp/language/copy_elision#Non-mandatory_elision_of_copy.2Fmove_.28since_C.2B.2B11.29_operationsAs far as whether or not you need to use std::move(), that only involves (1), and is true for parameters as well. The only thing that changes in C++17 is whether (2) always eliminates the copy/move. (3) doesn't change, although it's generally safe to assume it will be eliminated in optimized builds (but it's not portable to assume side effects are always eliminated).
It's probably worthwhile to discuss these topics in at least some detail.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
K. Moon (Gerrit)
Patchset:
It's subtle, and I had to double-check this, but we need to distinguish between three cases here: […]
For reference, I played around with the different cases in Compiler Explorer:
https://godbolt.org/z/bbdnM5rhh
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Nico Weber (Gerrit)
Attention is currently required from: K. Moon, Martin Bidlingmaier.
Patch set 6:Code-Review +1
1 comment:
Patchset:
For reference, I played around with the different cases in Compiler Explorer: […]
The interesting bit here is that the local is a subclass of the return type, no? ie https://godbolt.org/z/ea31TqcG7
I thought this would be affected by https://cplusplus.github.io/CWG/issues/1579.html and hence c++14 is what makes the difference, but apparently clang trunk doesn't consider the std::move() pessimizing even in c++11.
But it works without the move too.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
K. Moon (Gerrit)
Attention is currently required from: Martin Bidlingmaier.
1 comment:
Patchset:
The interesting bit here is that the local is a subclass of the return type, no? ie https://godbolt. […]
Agreed; it seems like more could be done here, but the change is fine as-is.
To view, visit change 3788415. To unsubscribe, or for help writing mail filters, visit settings.
Martin Bidlingmaier (Gerrit)
Patch set 6:Commit-Queue +2
chromium-website-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
chromium-we...@luci-project-accounts.iam.gserviceaccount.com submitted this change.
Remove paragraph on return std::move with implicit conversion
The std::move is no longer needed in these situations.
Drive-by: Replace unique_ptr<...>(new ...) with make_unique in code snippet.
Bug: b/240531273
Change-Id: I17d34b011eedd906552a6bf8583a021e9071928a
Reviewed-on: https://chromium-review.googlesource.com/c/website/+/3788415
Reviewed-by: Nico Weber <tha...@chromium.org>
Commit-Queue: Martin Bidlingmaier <mb...@google.com>
Reviewed-by: Elly Fong-Jones <elly...@chromium.org>
---
M site/developers/smart-pointer-guidelines/index.md
M site/rvalue-references/index.md
2 files changed, 21 insertions(+), 24 deletions(-)