[BUG] Emitted code relies on implicit move and copy constructors, but move and copy constructors are marked explicit

[cpp-niel]

Describe the bug
cppfront is emitting the explicit specifier on all single argument constructors including move and copy constructors. At the same time implicit move and copy are being used by code that is emitted - for instance - when returning named values from functions.

To Reproduce
Steps to reproduce the behavior:

1. Sample code - distilled down to minimal essentials please
   Here is a Compiler Explorer link demonstrating the problem: https://godbolt.org/z/x8T4ec7Wd
   Note how foo's move and copy constructors are explicit, but the call to get_foo() in main would need at least an implicit copy constructor.
2. Command lines including which C++ compiler you are using
   Calling cppfront with -p and compiling the result with gcc-12
3. Expected result - what you expected to happen
   I would expect copy and move constructors to not be explicit
4. Actual result/error6.
   Copy and move constructors are marked explicit

Additional context
I have resolved this issue locally by only emitting the explicit specifier if the constructor does not have a that:

diff --git a/source/cppfront.cpp b/source/cppfront.cpp
index 570c078..e4e2a04 100644
--- a/source/cppfront.cpp
+++ b/source/cppfront.cpp
@@ -5139,6 +5139,7 @@ public:
                         break;default:
                             if (
                                 func->is_constructor()
+                                && !func->is_constructor_with_that()
                                 && func->parameters->ssize() == 2
                                 && generating_assignment_from != &n
                                 )

[filipsajdak]

There are at least two issues here.

1. explicit constructor suppress copy-elision (NRVO) -> https://godbolt.org/z/jfdaWGdh4 (this is disappointing),
2. the move-from-last use also suppress copy-elision (NRVO),

Both issues are harmful as they impact performance without bringing any value.

Just because the performance is already broken, you can overcome the current limitations by explicitly calling a constructor - the impact will be the same as with an implicit constructor and move-from-last.

I'm not too fond of making constructors implicit by default - I hope we can find a solution that will not suppress copy elision. We can mimic copy elision on the cpp2 side, but it will impact the cpp1 code.

[JohelEGP]

2. he move-from-last use also suppress copy-elision (NRVO),

And interferes with implicit moves, which actually does OR, and so the explicit move can result in a hard error.

[hsutter]

Re constructors: My intent was that explicit should just be the default for converting constructors, not for copy/move constructors. So it should be removed for copy/move constructors. That would address your problem, right? And I think that's exactly what your proposed change would do.

Re move from last use: That should be suppressed when the last use is the beginning of a return expression, right? (Somehow I thought I already did that, but I guess not.)

[JohelEGP]

Re move from last use: That should be suppressed when the last use is the beginning of a return expression, right? (Somehow I thought I already did that, but I guess not.)

Only when the operand is exactly the name of a non-parameter local.

[Note 2: A return statement can involve an invocation of a constructor to perform a copy or move of the operand if it is not a prvalue or if its type differs from the return type of the function.
A copy operation associated with a return statement can be elided or converted to a move operation if an automatic storage duration variable is returned ([class.copy.elision]).
— end note]

(1.1)
in a return statement in a function with a class return type, when the expression is the name of a non-volatile object with automatic storage duration (other than a function parameter or a variable introduced by the exception-declaration of a handler ([except.handle])) with the same type (ignoring cv-qualification) as the function return type, the copy/move operation can be omitted by constructing the object directly into the function call's return object

I remember reading that implementations reliably apply this optimization.

[cpp-niel]

Re constructors: My intent was that explicit should just be the default for converting constructors, not for copy/move constructors. So it should be removed for copy/move constructors. That would address your problem, right? And I think that's exactly what your proposed change would do.

Yes, that would address the problem and is what I was trying to achieve with the proposed change. If it's the right thing to do, I'd be happy to create a pull request with the change and an accompanying test case?

[hsutter]

I was making another change anyway so I just did it while I was in the file. Thanks again for pointing this out!