diff mbox

PATCH to implement C++14 VLA semantics

Message ID 518BD19E.7080500@redhat.com
State New
Headers show

Commit Message

Jason Merrill May 9, 2013, 4:41 p.m. UTC 
At the last C++ standards meeting, we agreed to add VLAs to the 
language.  But they're significantly different from GNU/C99 VLAs: you 
can't form a pointer to a VLA, or take its sizeof, or really anything 
other than directly use it.  We also need to throw an exception if we 
try to create one with a negative or too large bound.  And we need to 
support lambda capture and range-based for loops.

The one thing I'm nervous about is our handling of an array which turns 
out at runtime to be of length 0.  GCC has always allowed zero-length 
arrays, and they work fine, so I don't want to break existing code, but 
I also want to offer a fully-conforming mode.  What I ended up deciding 
to do is throw on zero with -std=c++1y and not with -std=gnu++1y.  But 
I'm not terribly comfortable with this answer; anyone have any better ideas?

The first patch accepts VLAs in C++1y mode and adds new functionality; 
the second patch adds diagnostics for invalid uses of C++1y VLAs.

Tested x86_64-pc-linux-gnu, applying to trunk.

Comments

Gabriel Dos Reis May 9, 2013, 4:57 p.m. UTC | #1 
On Thu, May 9, 2013 at 11:41 AM, Jason Merrill <jason@redhat.com> wrote:
> At the last C++ standards meeting, we agreed to add VLAs to the language.
> But they're significantly different from GNU/C99 VLAs: you can't form a
> pointer to a VLA, or take its sizeof, or really anything other than directly
> use it.  We also need to throw an exception if we try to create one with a
> negative or too large bound.  And we need to support lambda capture and
> range-based for loops.
>
> The one thing I'm nervous about is our handling of an array which turns out
> at runtime to be of length 0.  GCC has always allowed zero-length arrays,
> and they work fine, so I don't want to break existing code, but I also want
> to offer a fully-conforming mode.  What I ended up deciding to do is throw
> on zero with -std=c++1y and not with -std=gnu++1y.  But I'm not terribly
> comfortable with this answer; anyone have any better ideas?

I am fine with this distinction.  I suspect that we don't have
too many codes with VLA array with zero length in existing C++ mode.


>
> The first patch accepts VLAs in C++1y mode and adds new functionality; the
> second patch adds diagnostics for invalid uses of C++1y VLAs.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
Florian Weimer May 13, 2013, 12:25 p.m. UTC | #2 
On 05/09/2013 06:41 PM, Jason Merrill wrote:
> At the last C++ standards meeting, we agreed to add VLAs to the
> language.  But they're significantly different from GNU/C99 VLAs: you
> can't form a pointer to a VLA, or take its sizeof, or really anything
> other than directly use it.  We also need to throw an exception if we
> try to create one with a negative or too large bound.

I'm not sure if we should throw the exception in case of large size_t 
values.  Even with the checks in place, there is still a wide gap where 
the definition triggers undefined behavior due to stack overflow.

This whole feature seems rather poorly designed to me.  The code size 
increase due to official VLA support in C++11y might come a bit as a 
surprise.  But rereading N3639, there's no way around it, at least for 
expressions of signed types.
Gabriel Dos Reis May 13, 2013, 1:06 p.m. UTC | #3 
On Mon, May 13, 2013 at 7:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 05/09/2013 06:41 PM, Jason Merrill wrote:
>>
>> At the last C++ standards meeting, we agreed to add VLAs to the
>> language.  But they're significantly different from GNU/C99 VLAs: you
>> can't form a pointer to a VLA, or take its sizeof, or really anything
>> other than directly use it.  We also need to throw an exception if we
>> try to create one with a negative or too large bound.
>
>
> I'm not sure if we should throw the exception in case of large size_t
> values.  Even with the checks in place, there is still a wide gap where the
> definition triggers undefined behavior due to stack overflow.
>
> This whole feature seems rather poorly designed to me.  The code size
> increase due to official VLA support in C++11y might come a bit as a
> surprise.  But rereading N3639, there's no way around it, at least for
> expressions of signed types.

I think there is a general mood of unsympathetic views towards liberal
"undefined behavior."  Of course, implementations are always free to
offer switches to programmers who don't want checks.

-- Gaby
Florian Weimer May 13, 2013, 1:09 p.m. UTC | #4 
On 05/13/2013 03:06 PM, Gabriel Dos Reis wrote:
>> This whole feature seems rather poorly designed to me.  The code size
>> increase due to official VLA support in C++11y might come a bit as a
>> surprise.  But rereading N3639, there's no way around it, at least for
>> expressions of signed types.
>
> I think there is a general mood of unsympathetic views towards liberal
> "undefined behavior."  Of course, implementations are always free to
> offer switches to programmers who don't want checks.

And usually I'm in that crowd as well.  But in this case, we add a check 
which only covers a tiny fraction of the problem.  It's like bounds 
checking for arrays which only fails if the index is at least twice as 
large as the array length, IMHO.
Jason Merrill May 13, 2013, 2:35 p.m. UTC | #5 
On 05/13/2013 09:09 AM, Florian Weimer wrote:
> And usually I'm in that crowd as well.  But in this case, we add a check
> which only covers a tiny fraction of the problem.  It's like bounds
> checking for arrays which only fails if the index is at least twice as
> large as the array length, IMHO.

The document is still in flux; if you have ideas about ways to improve 
the specification, I would be happy to submit them as a comment on the 
public draft.

Jason
diff mbox

Patch

commit 145fead03a44496158ccf901b9ead175db550282
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Apr 25 17:53:42 2013 -0400

    	N3639 C++1y VLA diagnostics
    
    	* decl.c (grokdeclarator): Complain about reference, pointer, or
    	typedef to VLA.
    	(create_array_type_for_decl): Complain about array of VLA.
    	* pt.c (tsubst): Likewise.
    	* rtti.c (get_tinfo_decl): Talk about "array of runtime bound".
    	* semantics.c (finish_decltype_type): Complain about decltype of VLA.
    	* typeck.c (cp_build_addr_expr_1): Complain about VLA.
    	(cxx_sizeof_or_alignof_type): Likewise.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index b762d28..a3250a2 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8478,6 +8478,9 @@  create_array_type_for_decl (tree name, tree type, tree size)
       return error_mark_node;
     }
 
+  if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+    pedwarn (input_location, OPT_Wvla, "array of array of runtime bound");
+
   /* Figure out the index type for the array.  */
   if (size)
     itype = compute_array_index_type (name, size, tf_warning_or_error);
@@ -9718,6 +9721,12 @@  grokdeclarator (const cp_declarator *declarator,
                    : G_("cannot declare pointer to qualified function type %qT"),
 		   type);
 
+	  if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+	    pedwarn (input_location, OPT_Wvla,
+		     declarator->kind == cdk_reference
+		     ? G_("reference to array of runtime bound")
+		     : G_("pointer to array of runtime bound"));
+
 	  /* When the pointed-to type involves components of variable size,
 	     care must be taken to ensure that the size evaluation code is
 	     emitted early enough to dominate all the possible later uses
@@ -10072,6 +10081,10 @@  grokdeclarator (const cp_declarator *declarator,
 	  type = error_mark_node;
 	}
 
+      if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+	pedwarn (input_location, OPT_Wvla,
+		 "typedef naming array of runtime bound");
+
       if (decl_context == FIELD)
 	decl = build_lang_decl (TYPE_DECL, unqualified_id, type);
       else
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index dca3407..2cb2abd 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11560,6 +11560,18 @@  tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	  r = cp_build_reference_type (type, TYPE_REF_IS_RVALUE (t));
 	r = cp_build_qualified_type_real (r, cp_type_quals (t), complain);
 
+	if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+	  {
+	    if (complain & tf_warning_or_error)
+	      pedwarn
+		(input_location, OPT_Wvla,
+		 code == REFERENCE_TYPE
+		 ? G_("cannot declare reference to array of runtime bound")
+		 : G_("cannot declare pointer to array of runtime bound"));
+	    else
+	      r = error_mark_node;
+	  }
+
 	if (r != error_mark_node)
 	  /* Will this ever be needed for TYPE_..._TO values?  */
 	  layout_type (r);
diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index 4e73165..9010440 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -393,9 +393,12 @@  get_tinfo_decl (tree type)
 
   if (variably_modified_type_p (type, /*fn=*/NULL_TREE))
     {
-      error ("cannot create type information for type %qT because "
-	     "it involves types of variable size",
-	     type);
+      if (array_of_runtime_bound_p (type))
+	error ("typeid of array of runtime bound");
+      else
+	error ("cannot create type information for type %qT because "
+	       "it involves types of variable size",
+	       type);
       return error_mark_node;
     }
 
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 27e7822..ff8ac2a 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5456,6 +5456,15 @@  finish_decltype_type (tree expr, bool id_expression_or_member_access_p,
 	}
     }
 
+  if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+    {
+      if (complain & tf_warning_or_error)
+	pedwarn (input_location, OPT_Wvla,
+		 "taking decltype of array of runtime bound");
+      else
+	return error_mark_node;
+    }
+
   return type;
 }
 
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 47670f2..df5fc4a 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1547,6 +1547,15 @@  cxx_sizeof_or_alignof_type (tree type, enum tree_code op, bool complain)
       return value;
     }
 
+  if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
+    {
+      if (complain & tf_warning_or_error)
+	pedwarn (input_location, OPT_Wvla,
+		 "taking sizeof array of runtime bound");
+      else
+	return error_mark_node;
+    }
+
   return c_sizeof_or_alignof_type (input_location, complete_type (type),
 				   op == SIZEOF_EXPR,
 				   complain);
@@ -5316,7 +5325,17 @@  cp_build_addr_expr_1 (tree arg, bool strict_lvalue, tsubst_flags_t complain)
     }
 
   if (argtype != error_mark_node)
-    argtype = build_pointer_type (argtype);
+    {
+      if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (argtype))
+	{
+	  if (complain & tf_warning_or_error)
+	    pedwarn (input_location, OPT_Wvla,
+		     "taking address of array of runtime bound");
+	  else
+	    return error_mark_node;
+	}
+      argtype = build_pointer_type (argtype);
+    }
 
   /* In a template, we are processing a non-dependent expression
      so we can just form an ADDR_EXPR with the correct type.  */
diff --git a/gcc/testsuite/g++.dg/cpp1y/vla1.C b/gcc/testsuite/g++.dg/cpp1y/vla1.C
new file mode 100644
index 0000000..29a59ed
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/vla1.C
@@ -0,0 +1,40 @@ 
+// { dg-options "-std=c++1y -pedantic-errors" }
+
+#include <typeinfo>
+
+void f(int n)
+{
+  int a[n];
+  int aa[n][n];			// { dg-error "" }
+  &a;				// { dg-error "" }
+  sizeof a;			// { dg-error "" }
+  typeid(a);			// { dg-error "" }
+  decltype(a) a2;		// { dg-error "" }
+  typedef int at[n];		// { dg-error "" }
+  int (*p)[n];			// { dg-error "" }
+  int (&r)[n] = a;		// { dg-error "" }
+  struct A
+  {
+    int a[n];			// { dg-error "" }
+  };
+}
+
+template <class T>
+void g(int n)
+{
+  int a[n];
+  int aa[n][n];			// { dg-error "" }
+  &a;				// { dg-error "" }
+  sizeof a;			// { dg-error "" }
+  typeid(a);			// { dg-error "" }
+  decltype(a) a2;		// { dg-error "" }
+  typedef int at[n];		// { dg-error "" }
+  int (*p)[n];			// { dg-error "" }
+  int (&r)[n] = a;		// { dg-error "" }
+  struct A
+  {
+    int a[n];			// { dg-error "" }
+  };
+}
+
+template void g<int>(int);