[C++] Fix comment

In answering a question about passing non-trivial types through ..., I 
discovered a misleading comment.  It is NOT just like a value parm, 
because we don't locally copy it.

class X { ..,non-poddy... };
void Foo (X, ...);
void bin (X &p)
{
   Foo (p, p);
}
Only the first arg to Foo goes via a copy-constructor.

Fixed thusly and applied.

nathan

On Wed, Oct 4, 2017 at 10:12 AM, Nathan Sidwell <nathan@acm.org> wrote:
> In answering a question about passing non-trivial types through ..., I
> discovered a misleading comment.  It is NOT just like a value parm, because
> we don't locally copy it.

Hmm, I think the error is in the behavior, not the comment.  :)

Jason

On 10/04/2017 11:29 AM, Jason Merrill wrote:
> On Wed, Oct 4, 2017 at 10:12 AM, Nathan Sidwell <nathan@acm.org> wrote:
>> In answering a question about passing non-trivial types through ..., I
>> discovered a misleading comment.  It is NOT just like a value parm, because
>> we don't locally copy it.
> 
> Hmm, I think the error is in the behavior, not the comment.  :)

I wouldn't disagree.

nathan

On Wed, Oct 4, 2017 at 11:44 AM, Nathan Sidwell <nathan@acm.org> wrote:
> On 10/04/2017 11:29 AM, Jason Merrill wrote:
>> On Wed, Oct 4, 2017 at 10:12 AM, Nathan Sidwell <nathan@acm.org> wrote:
>>> In answering a question about passing non-trivial types through ..., I
>>> discovered a misleading comment.  It is NOT just like a value parm,
>>> because
>>> we don't locally copy it.
>>
>> Hmm, I think the error is in the behavior, not the comment.  :)
>
> I wouldn't disagree.

Fixing thus.
commit e7df49dcd6facccacb13888d44896dae7d3811fa
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Oct 5 15:12:11 2017 -0400

            * call.c (convert_arg_to_ellipsis): Use the result of force_rvalue.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index bfd92882393..9d747be9d79 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7165,29 +7165,24 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
       /* In a template (or ill-formed code), we can have an incomplete type
 	 even after require_complete_type_sfinae, in which case we don't know
 	 whether it has trivial copy or not.  */
-      && COMPLETE_TYPE_P (arg_type))
+      && COMPLETE_TYPE_P (arg_type)
+      && !cp_unevaluated_operand)
     {
-      /* Build up a real lvalue-to-rvalue conversion in case the
-	 copy constructor is trivial but not callable.  */
-      if (!cp_unevaluated_operand && CLASS_TYPE_P (arg_type))
-	force_rvalue (arg, complain);
-
       /* [expr.call] 5.2.2/7:
 	 Passing a potentially-evaluated argument of class type (Clause 9)
 	 with a non-trivial copy constructor or a non-trivial destructor
 	 with no corresponding parameter is conditionally-supported, with
 	 implementation-defined semantics.
 
-	 We support it as pass-by-invisible-reference to the caller's
-	 object.  That's different to named by-value parameters, which
-	 construct a copy and pass a reference to that.
+	 We support it as pass-by-invisible-reference, just like a normal
+	 value parameter.
 
 	 If the call appears in the context of a sizeof expression,
 	 it is not potentially-evaluated.  */
-      if (cp_unevaluated_operand == 0
-	  && (type_has_nontrivial_copy_init (arg_type)
-	      || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (arg_type)))
+      if (type_has_nontrivial_copy_init (arg_type)
+	  || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (arg_type))
 	{
+	  arg = force_rvalue (arg, complain);
 	  if (complain & tf_warning)
 	    warning (OPT_Wconditionally_supported,
 		     "passing objects of non-trivially-copyable "
@@ -7195,6 +7190,11 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
 		     arg_type);
 	  return cp_build_addr_expr (arg, complain);
 	}
+      /* Build up a real lvalue-to-rvalue conversion in case the
+	 copy constructor is trivial but not callable.  */
+      else if (CLASS_TYPE_P (arg_type))
+	force_rvalue (arg, complain);
+
     }
 
   return arg;
diff --git a/gcc/testsuite/g++.dg/ext/varargs2.C b/gcc/testsuite/g++.dg/ext/varargs2.C
new file mode 100644
index 00000000000..13ddf5bb527
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/varargs2.C
@@ -0,0 +1,17 @@
+// { dg-do run }
+
+int c;
+struct X { X() {}; X(const X&) { ++c; } };
+void Foo (X, ...) {}
+void bin (X &p)
+{
+  Foo (p, p);
+}
+
+int main()
+{
+  X x;
+  bin(x);
+  if (c != 2)
+    __builtin_abort();
+}

On 10/05/2017 05:25 PM, Jason Merrill wrote:
> On Wed, Oct 4, 2017 at 11:44 AM, Nathan Sidwell <nathan@acm.org> wrote:
>> On 10/04/2017 11:29 AM, Jason Merrill wrote:
>>> On Wed, Oct 4, 2017 at 10:12 AM, Nathan Sidwell <nathan@acm.org> wrote:

>>> Hmm, I think the error is in the behavior, not the comment.  :)
>>
>> I wouldn't disagree.
> 
> Fixing thus.

Nice.  This is a visible ABI change, so might want to add it to 8.1's 
release notes?
1) variadic fns can no longer mutate the caller's object
2) variadic fns now observe the static type of the object, not the 
dynamic type.

(and if any user's relying on that, it's their problem :)

nathan