Patchwork Go patch committed: Don't crash ranging over call to builtin function

login
register
mail settings
Submitter Ian Taylor
Date May 11, 2011, 7:39 p.m.
Message ID <mcrpqnpdo2c.fsf@coign.corp.google.com>
Download mbox | patch
Permalink /patch/95197/
State New
Headers show

Comments

Ian Taylor - May 11, 2011, 7:39 p.m.
This patch to the Go frontend fixes a compiler crash for code like
	for i, v := range append(a, b...) {
	}
The problem is that for a case like this there are multiple pointers to
the call expression, which interacts poorly with lowering in which the
call expression is replaced.  This patch papers over the problem.  It's
not a particularly clean fix but I can't think of any way that it will
actually fail.  The patch also includes a couple of minor cleanups.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r ddb4e0e9b3e8 go/expressions.cc
--- a/go/expressions.cc	Fri May 06 17:09:43 2011 -0700
+++ b/go/expressions.cc	Wed May 11 11:38:53 2011 -0700
@@ -8527,9 +8527,9 @@ 
     new_args->push_back(Expression::make_nil(loc));
 
   // We can't return a new call expression here, because this one may
-  // be referenced by Call_result expressions.  FIXME.
-  if (old_args != NULL)
-    delete old_args;
+  // be referenced by Call_result expressions.  FIXME.  We can't
+  // delete OLD_ARGS because we may have both a Call_expression and a
+  // Builtin_call_expression which refer to them.  FIXME.
   this->args_ = new_args;
   this->varargs_are_lowered_ = true;
 
@@ -9250,8 +9250,8 @@ 
 	  error_at(location, "invalid slice of map");
 	  return Expression::make_error(location);
 	}
-      Map_index_expression* ret= Expression::make_map_index(left, start,
-							    location);
+      Map_index_expression* ret = Expression::make_map_index(left, start,
+							     location);
       if (this->is_lvalue_)
 	ret->set_is_lvalue();
       return ret;
diff -r ddb4e0e9b3e8 go/statements.cc
--- a/go/statements.cc	Fri May 06 17:09:43 2011 -0700
+++ b/go/statements.cc	Wed May 11 11:38:53 2011 -0700
@@ -4536,7 +4536,7 @@ 
   else
     {
       this->report_error(_("range clause must have "
-			   "array, slice, setring, map, or channel type"));
+			   "array, slice, string, map, or channel type"));
       return Statement::make_error_statement(this->location());
     }
 
@@ -4552,6 +4552,7 @@ 
     {
       range_temp = Statement::make_temporary(NULL, this->range_, loc);
       temp_block->add_statement(range_temp);
+      this->range_ = NULL;
     }
 
   Temporary_statement* index_temp = Statement::make_temporary(index_type,