This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFC: PATCH to allow passing non-trivial types through ...


A customer was recently complaining about G++ rejecting code that tries to pass a type with a non-trivial copy constructor through ..., which is undefined in C++98 and conditionally-supported in C++11. In GCC 3.1 and below we gave a warning and then did a bitwise copy. From GCC 3.2 to 4.4 we gave a warning and then aborted at runtime. From 4.5 on we have just given an error.

On considering the request, it occurred to me that we could handle variadic arguments of non-trivial types the same way we handle normal value arguments of such types: pass by invisible reference. So this patch implements that. Since it's been so long since this was allowed at all, I don't think we need to worry about ABI incompatibility with the 3.1 behavior.

Thoughts?
commit 68a0f48b2c82f157a04842e996fe340428fcfcc3
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Aug 8 17:23:14 2014 -0400

    	* call.c (build_x_va_arg): Support passing non-POD through ....
    	(convert_arg_to_ellipsis): Likewise.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 56b3c5a..fbe92a9 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6570,8 +6570,8 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
 	 with no corresponding parameter is conditionally-supported, with
 	 implementation-defined semantics.
 
-	 We used to just warn here and do a bitwise copy, but now
-	 cp_expr_size will abort if we try to do 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.  */
@@ -6579,10 +6579,12 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
 	  && (type_has_nontrivial_copy_init (arg_type)
 	      || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (arg_type)))
 	{
-	  if (complain & tf_error)
-	    error_at (loc, "cannot pass objects of non-trivially-copyable "
-		      "type %q#T through %<...%>", arg_type);
-	  return error_mark_node;
+	  if (complain & tf_warning)
+	    warning (OPT_Wconditionally_supported,
+		     "passing objects of non-trivially-copyable "
+		     "type %q#T through %<...%> is conditionally supported",
+		     arg_type);
+	  return cp_build_addr_expr (arg, complain);
 	}
     }
 
@@ -6595,7 +6597,11 @@ tree
 build_x_va_arg (source_location loc, tree expr, tree type)
 {
   if (processing_template_decl)
-    return build_min (VA_ARG_EXPR, type, expr);
+    {
+      tree r = build_min (VA_ARG_EXPR, type, expr);
+      SET_EXPR_LOCATION (r, loc);
+      return r;
+    }
 
   type = complete_type_or_else (type, NULL_TREE);
 
@@ -6604,18 +6610,24 @@ build_x_va_arg (source_location loc, tree expr, tree type)
 
   expr = mark_lvalue_use (expr);
 
+  if (TREE_CODE (type) == REFERENCE_TYPE)
+    {
+      error ("cannot receive reference type %qT through %<...%>", type);
+      return error_mark_node;
+    }
+
   if (type_has_nontrivial_copy_init (type)
-      || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
-      || TREE_CODE (type) == REFERENCE_TYPE)
+      || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
     {
-      /* Remove reference types so we don't ICE later on.  */
-      tree type1 = non_reference (type);
-      /* conditionally-supported behavior [expr.call] 5.2.2/7.  */
-      error ("cannot receive objects of non-trivially-copyable type %q#T "
-	     "through %<...%>; ", type);
-      expr = convert (build_pointer_type (type1), null_node);
-      expr = cp_build_indirect_ref (expr, RO_NULL, tf_warning_or_error);
-      return expr;
+      /* conditionally-supported behavior [expr.call] 5.2.2/7.  Let's treat
+	 it as pass by invisible reference.  */
+      warning_at (loc, OPT_Wconditionally_supported,
+		 "receiving objects of non-trivially-copyable type %q#T "
+		 "through %<...%> is conditionally-supported", type);
+
+      tree ptr = build_pointer_type (type);
+      ptr = build_va_arg (loc, expr, ptr);
+      return cp_build_indirect_ref (ptr, RO_NULL, tf_warning_or_error);
     }
 
   return build_va_arg (loc, expr, type);
diff --git a/gcc/doc/implement-cxx.texi b/gcc/doc/implement-cxx.texi
index 50efcc3..5802311 100644
--- a/gcc/doc/implement-cxx.texi
+++ b/gcc/doc/implement-cxx.texi
@@ -42,7 +42,9 @@ all conditionally-supported constructs that it does not support (C++0x
 @cite{Whether an argument of class type with a non-trivial copy
 constructor or destructor can be passed to ... (C++0x 5.2.2).}
 
-Such argument passing is not supported.
+Such argument passing is supported, using the same
+pass-by-invisible-reference approach used for normal function
+arguments of such types.
 
 @end itemize
 
diff --git a/gcc/testsuite/g++.dg/ext/varargs1.C b/gcc/testsuite/g++.dg/ext/varargs1.C
new file mode 100644
index 0000000..b67d788
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/varargs1.C
@@ -0,0 +1,34 @@
+// Test that passing an object with non-trivial copy constructor and
+// destructor is (conditionally) supported and has sensible semantics.
+
+#include <stdarg.h>
+extern "C" void abort();
+
+void *as[5];
+int i;
+
+struct A {
+  A() { as[i++] = this; }
+  A(const A& a) {
+    if (&a != as[i-1])
+      abort();
+    as[i++] = this;
+  }
+  ~A() {
+    if (this != as[--i])
+      abort();
+  }
+};
+
+void f(int i, ...) {
+  va_list ap;
+  va_start (ap, i);
+  A ar = va_arg (ap, A);
+}
+
+int main()
+{
+  f(42,A());
+  if (i != 0)
+    abort();
+}
diff --git a/gcc/testsuite/g++.dg/overload/ellipsis1.C b/gcc/testsuite/g++.dg/overload/ellipsis1.C
index 3dedaa6..1dde2bc 100644
--- a/gcc/testsuite/g++.dg/overload/ellipsis1.C
+++ b/gcc/testsuite/g++.dg/overload/ellipsis1.C
@@ -1,5 +1,6 @@
 // PR c++/15142
 // Bug: We were aborting after giving a warning about passing a non-POD.
+// { dg-options "-Wconditionally-supported" }
 
 struct B { 
     B() throw() { } 
@@ -14,5 +15,5 @@ struct X {
 struct S { S(...); }; 
  
 void SillyFunc() { 
-  throw S(X()); 		// { dg-error "copy" }
+  throw S(X()); 		// { dg-message "copy" }
 } 
diff --git a/gcc/testsuite/g++.dg/overload/ellipsis2.C b/gcc/testsuite/g++.dg/overload/ellipsis2.C
index d9118ba..c226e1c 100644
--- a/gcc/testsuite/g++.dg/overload/ellipsis2.C
+++ b/gcc/testsuite/g++.dg/overload/ellipsis2.C
@@ -1,4 +1,5 @@
 // PR c++/60253
+// { dg-options "-Wconditionally-supported" }
 
 struct A
 {
@@ -10,4 +11,4 @@ struct B
   B(...);
 };
 
-B b(0, A());  // { dg-error "cannot pass" }
+B b(0, A());  // { dg-message "pass" }
diff --git a/gcc/testsuite/g++.dg/warn/var-args1.C b/gcc/testsuite/g++.dg/warn/var-args1.C
index 9bd84a7..35deb09 100644
--- a/gcc/testsuite/g++.dg/warn/var-args1.C
+++ b/gcc/testsuite/g++.dg/warn/var-args1.C
@@ -6,6 +6,6 @@ void foo(int, ...)
 {
     va_list va;
     int i;
-    i = va_arg(va, int&); /* { dg-error "cannot receive objects" } */
+    i = va_arg(va, int&); /* { dg-error "cannot receive" } */
 }
 
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/crash63.C b/gcc/testsuite/g++.old-deja/g++.brendan/crash63.C
index 89685fc..badd926 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/crash63.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/crash63.C
@@ -13,4 +13,4 @@ class UnitList
    UnitList (...);
    };
 
-UnitList unit_list (String("keV")); // { dg-error "" } cannot pass non-pod
+UnitList unit_list (String("keV"));
diff --git a/gcc/testsuite/g++.old-deja/g++.other/vaarg3.C b/gcc/testsuite/g++.old-deja/g++.other/vaarg3.C
index 134a89c..98f7877 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/vaarg3.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/vaarg3.C
@@ -1,5 +1,6 @@
 // { dg-do assemble  }
-// { dg-options "-Wno-abi" { target arm_eabi } }
+// { dg-options "-Wconditionally-supported" }
+// { dg-options "-Wno-abi -Wconditionally-supported" { target arm_eabi } }
 
 // Copyright (C) 1999 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 4 Oct 1999 <nathan@acm.org>
@@ -14,19 +15,19 @@ struct Z;   // { dg-message "forward decl" }
 void fn1(va_list args)
 {
   int i = va_arg (args, int);
-  Y x = va_arg (args, Y);         // { dg-error "cannot receive" }
-  Y y = va_arg (args, struct Y);  // { dg-error "cannot receive" }
+  Y x = va_arg (args, Y);         // { dg-message "receiv" }
+  Y y = va_arg (args, struct Y);  // { dg-message "receiv" }
   int &r = va_arg (args, int &);  // { dg-error "cannot receive" }
   
   Z z1 = va_arg (args, Z);        // { dg-error "incomplete" } 
   const Z &z2 = va_arg (args, Z);       // { dg-error "incomplete" } 
 
   va_arg (args, char);    // { dg-warning "promote" } 
-  // { dg-message "should pass" "pass" { target *-*-* } 24 }
-  // { dg-message "abort" "abort" { target *-*-* } 24 }
+  // { dg-message "should pass" "pass" { target *-*-* } 25 }
+  // { dg-message "abort" "abort" { target *-*-* } 25 }
   va_arg (args, int []);  // { dg-error "array with unspecified bounds" } promote
   va_arg (args, int ());  // { dg-warning "promoted" } promote
-  // { dg-message "abort" "abort" { target *-*-* } 28 }
+  // { dg-message "abort" "abort" { target *-*-* } 29 }
   va_arg (args, bool);    // { dg-warning "promote" "promote" } 
-  // { dg-message "abort" "abort" { target *-*-* } 30 }
+  // { dg-message "abort" "abort" { target *-*-* } 31 }
 }
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C b/gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C
index e880119..ee84db9 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/vaarg3.C
@@ -1,4 +1,5 @@
 // { dg-do assemble  }
+// { dg-options "-Wconditionally-supported" }
 // Copyright (C) 2000 Free Software Foundation
 // Contributed by Nathan Sidwell 22 June 2000 <nathan@codesourcery.com>
 
@@ -14,14 +15,14 @@ void PrintArgs (Type somearg, ...)
 va_list argp;
 va_start (argp, somearg);
 Type value;
-value = va_arg (argp, Type); // { dg-error "cannot receive" } cannot pass non-POD
+value = va_arg (argp, Type); // { dg-message "receiv" } cannot pass non-POD
 va_end (argp);
 }
 
 int main (void)
 {
 A dummy;
-PrintArgs (dummy, dummy); // { dg-error "cannot pass" } cannot pass non-POD
-// { dg-message "required" "inst" { target *-*-* } 24 }
+PrintArgs (dummy, dummy); // { dg-message "pass" } cannot pass non-POD
+// { dg-message "required" "inst" { target *-*-* } 25 }
 return 0;
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]