code review 191970043: compiler: Don't construct arrays of zero-sized values.

compiler: Don't construct arrays of zero-sized values.

Fixes gcc.gnu.org/PR61265.

[cmang, 2014-12-17 22:00:03]

Hello iant@golang.org (cc: gofrontend-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/gofrontend

[iant, 2014-12-19 04:07:25]

https://codereview.appspot.com/191970043/diff/40001/go/expressions.cc
File go/expressions.cc (right):

https://codereview.appspot.com/191970043/diff/40001/go/expressions.cc#newcode...
go/expressions.cc:12203: (*pv)->get_backend(context);
You aren't storing this anywhere, so it won't get evaluated.

In any case, I think it might be nicer to do this in go-gcc.cc.  Can we do it
there instead?

[cmang, 2014-12-19 15:21:22]

PTAL, this is the patch for gcc/go/go-gcc.cc that implements this:

Index: gcc/go/go-gcc.cc
======================================================================
--- a/gcc/go/go-gcc.cc
+++ b/gcc/go/go-gcc.cc
@@ -1688,16 +1688,20 @@ Gcc_backend::constructor_expression(Btype* btype,
 Bexpression*
 Gcc_backend::array_constructor_expression(
     Btype* array_btype, const std::vector<unsigned long>& indexes,
-    const std::vector<Bexpression*>& vals, Location)
+    const std::vector<Bexpression*>& vals, Location location)
 {
   tree type_tree = array_btype->get_tree();
   if (type_tree == error_mark_node)
     return this->error_expression();
 
   gcc_assert(indexes.size() == vals.size());
+
+  tree element_type = TREE_TYPE(type_tree);
+  size_t element_size = this->type_size(this->make_type(element_type));
   vec<constructor_elt, va_gc> *init;
-  vec_alloc(init, vals.size());
+  vec_alloc(init, element_size == 0 ? 0 : vals.size());
 
+  tree sink = NULL_TREE;
   bool is_constant = true;
   for (size_t i = 0; i < vals.size(); ++i)
     {
@@ -1708,6 +1712,19 @@ Gcc_backend::array_constructor_expression(
           || val == error_mark_node)
         return this->error_expression();
 
+      if (element_size == 0)
+	{
+	  // GIMPLE cannot represent arrays of zero-sized types so trying
+	  // to construct an array of zero-sized values might lead to errors.
+	  // Instead, we evaluate each expression that would have been added as
+	  // an array value for its side-effects and construct an empty array.
+	  if (sink == NULL_TREE)
+	    sink = val;
+	  else
+	    sink = build2(COMPOUND_EXPR, void_type_node, sink, val);
+	  continue;
+	}
+
       if (!TREE_CONSTANT(val))
         is_constant = false;
 
@@ -1720,6 +1737,9 @@ Gcc_backend::array_constructor_expression(
   tree ret = build_constructor(type_tree, init);
   if (is_constant)
     TREE_CONSTANT(ret) = 1;
+  if (sink != NULL_TREE)
+    ret = fold_build2_loc(location.gcc_location(), COMPOUND_EXPR,
+			  type_tree, sink, ret);
   return this->make_expression(ret);
 }

https://codereview.appspot.com/191970043/diff/40001/go/expressions.cc
File go/expressions.cc (right):

https://codereview.appspot.com/191970043/diff/40001/go/expressions.cc#newcode...
go/expressions.cc:12203: (*pv)->get_backend(context);
On 2014/12/19 04:07:25, iant wrote:
> You aren't storing this anywhere, so it won't get evaluated.
> 
> In any case, I think it might be nicer to do this in go-gcc.cc.  Can we do it
> there instead?

Done.

[iant, 2014-12-19 20:18:04]

On Fri, Dec 19, 2014 at 7:21 AM,  <cmang@golang.org> wrote:
>
> PTAL, this is the patch for gcc/go/go-gcc.cc that implements this:

Thanks.  I adjusted the patch to use int_size_in_bytes and
append_to_statement_list, as appended.  Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2014-12-19  Chris Manghane  <cmang@google.com>

	* go-gcc.cc (array_constructor_expression): Don't construct arrays
	of zero-sized values.